LinuxLists.cc - [PATCH 0/6] Initial Rust V4L2 support

[permalink] [raw]

Subject: [PATCH 1/6] rust: media: add the media module

As part of the initial rust v4l2 support, add the media module
to the kernel crate.

Signed-off-by: Daniel Almeida <[email protected]>
---
rust/kernel/lib.rs | 2 ++
rust/kernel/media/mod.rs | 5 +++++
rust/kernel/media/v4l2/mod.rs | 3 +++
3 files changed, 10 insertions(+)
create mode 100644 rust/kernel/media/mod.rs
create mode 100644 rust/kernel/media/v4l2/mod.rs

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c20b37e88ab2..d7e8f3297405 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -48,6 +48,8 @@ pub mod types;
#[doc(hidden)]
pub use bindings;
pub use macros;
+#[cfg(CONFIG_MEDIA_SUPPORT)]
+pub mod media;

#[cfg(CONFIG_ARM_AMBA)]
pub mod amba;
diff --git a/rust/kernel/media/mod.rs b/rust/kernel/media/mod.rs
new file mode 100644
index 000000000000..342e66382719
--- /dev/null
+++ b/rust/kernel/media/mod.rs
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Media subsystem
+
+pub mod v4l2;
diff --git a/rust/kernel/media/v4l2/mod.rs b/rust/kernel/media/v4l2/mod.rs
new file mode 100644
index 000000000000..068dd9b4863d
--- /dev/null
+++ b/rust/kernel/media/v4l2/mod.rs
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+//! Abstractions for include/media/v4l2-*.h
--
2.40.0

2023-04-06 21:58:52

[permalink] [raw]

Subject: [PATCH 3/6] rust: sync: introduce FfiMutex

Introduce an owned mutex that can be passed as bindings::mutex when
initializing C data structures. The kernel will be locking it without
user intervention so there is no data to protect and no guard type.

Signed-off-by: Daniel Almeida <[email protected]>
---
rust/kernel/sync.rs | 1 +
rust/kernel/sync/ffi_mutex.rs | 70 +++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+)
create mode 100644 rust/kernel/sync/ffi_mutex.rs

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index b07db83972ac..1415062be34d 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -26,6 +26,7 @@ use core::{cell::UnsafeCell, mem::MaybeUninit, pin::Pin};

mod arc;
mod condvar;
+pub mod ffi_mutex;
mod guard;
mod locked_by;
mod mutex;
diff --git a/rust/kernel/sync/ffi_mutex.rs b/rust/kernel/sync/ffi_mutex.rs
new file mode 100644
index 000000000000..e615ec4059f6
--- /dev/null
+++ b/rust/kernel/sync/ffi_mutex.rs
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! An owned mutex that can be passed as bindings::mutex when initializing C
+//! data structures. The kernel will be locking it without user intervention so
+//! there is no data to protect and no guard type.
+
+#![allow(dead_code)]
+
+use crate::prelude::*;
+use crate::sync::{LockClassKey, LockIniter};
+use core::marker::PhantomPinned;
+use core::pin::Pin;
+
+use crate::Opaque;
+
+/// An owned mutex that can be passed as bindings::mutex when initializing C
+/// data structures. The kernel will be locking it without user intervention so
+/// there is no data to protect and no guard type.
+pub struct FfiMutex {
+ mutex: Opaque<bindings::mutex>,
+ _pin: PhantomPinned,
+}
+
+impl FfiMutex {
+ /// Constructs a new Mutex for FFI purposes.
+ ///
+ /// # Safety
+ ///
+ /// The caller must call [`FfiMutex::init_lock`] before using the raw Mutex.
+ pub const unsafe fn new() -> Self {
+ Self {
+ mutex: Opaque::uninit(),
+ _pin: PhantomPinned,
+ }
+ }
+
+ /// Returns the inner bindings::mutex
+ ///
+ /// # Safety
+ ///
+ /// The caller must call [`FfiMutex::init_lock`] before using the raw Mutex.
+ pub(crate) unsafe fn raw(self: &mut Pin<&mut Self>) -> *mut bindings::mutex {
+ let this = self.as_mut();
+ // SAFETY: mutex is pinned when Lock is. The argument to the function is not moved.
+ let this = unsafe { this.map_unchecked_mut(|x| &mut x.mutex) };
+ // This does not move from the field.
+ this.get()
+ }
+}
+
+impl LockIniter for FfiMutex {
+ fn init_lock(self: Pin<&mut Self>, name: &'static CStr, key: &'static LockClassKey) {
+ unsafe { bindings::__mutex_init(self.mutex.get(), name.as_char_ptr(), key.get()) };
+ }
+}
+
+// SAFETY: the underlying bindings::mutex can be used from any thread.
+unsafe impl Send for FfiMutex {}
+// SAFETY: two threads can try locking the underlying bindings::mutex at the
+// same thread without issues.
+unsafe impl Sync for FfiMutex {}
+
+/// Safely initialises a [`FfiMutex`] with the given name, generating a new lock
+/// class.
+#[macro_export]
+macro_rules! ffi_mutex_init {
+ ($mutex:expr, $name:literal) => {
+ $crate::init_with_lockdep!($mutex, $name);
+ };
+}
--
2.40.0

2023-04-06 21:58:52

[permalink] [raw]

Subject: [PATCH 2/6] rust: media: add initial videodev2.h abstractions

Add initial videodev2.h abstractions.

Signed-off-by: Daniel Almeida <[email protected]>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/media/v4l2/capabilities.rs | 80 +++++++++++
rust/kernel/media/v4l2/enums.rs | 135 +++++++++++++++++++
rust/kernel/media/v4l2/format.rs | 178 +++++++++++++++++++++++++
rust/kernel/media/v4l2/framesize.rs | 176 ++++++++++++++++++++++++
rust/kernel/media/v4l2/inputs.rs | 104 +++++++++++++++
rust/kernel/media/v4l2/mmap.rs | 81 +++++++++++
rust/kernel/media/v4l2/mod.rs | 7 +
8 files changed, 762 insertions(+)
create mode 100644 rust/kernel/media/v4l2/capabilities.rs
create mode 100644 rust/kernel/media/v4l2/enums.rs
create mode 100644 rust/kernel/media/v4l2/format.rs
create mode 100644 rust/kernel/media/v4l2/framesize.rs
create mode 100644 rust/kernel/media/v4l2/inputs.rs
create mode 100644 rust/kernel/media/v4l2/mmap.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 048bae2197ac..3b3d6fcf110f 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -35,6 +35,7 @@
#include <linux/sysctl.h>
#include <linux/uaccess.h>
#include <linux/uio.h>
+#include <linux/videodev2.h>

/* `bindgen` gets confused at certain things. */
const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/media/v4l2/capabilities.rs b/rust/kernel/media/v4l2/capabilities.rs
new file mode 100644
index 000000000000..4abc5728f12d
--- /dev/null
+++ b/rust/kernel/media/v4l2/capabilities.rs
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Capabilities
+//!
+//! Part of the following C header: [`include/linux/videodev2.h`](../../../../include/linux/videodev2.h)
+
+/// Capabilities as defined by `V4L2_CAP_*`.
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum Capabilities {
+ VideoCapture = bindings::V4L2_CAP_VIDEO_CAPTURE as isize,
+ VideoOutput = bindings::V4L2_CAP_VIDEO_OUTPUT as isize,
+ VideoOverlay = bindings::V4L2_CAP_VIDEO_OVERLAY as isize,
+ VbiCapture = bindings::V4L2_CAP_VBI_CAPTURE as isize,
+ VbiOutput = bindings::V4L2_CAP_VBI_OUTPUT as isize,
+ SlicedVbiCapture = bindings::V4L2_CAP_SLICED_VBI_CAPTURE as isize,
+ SlicedVbiOutput = bindings::V4L2_CAP_SLICED_VBI_OUTPUT as isize,
+ RdsCapture = bindings::V4L2_CAP_RDS_CAPTURE as isize,
+ VideoOutputOverlay = bindings::V4L2_CAP_VIDEO_OUTPUT_OVERLAY as isize,
+ HwFrequencySeek = bindings::V4L2_CAP_HW_FREQ_SEEK as isize,
+ RdsOutput = bindings::V4L2_CAP_RDS_OUTPUT as isize,
+ VideoCaptureMplane = bindings::V4L2_CAP_VIDEO_CAPTURE_MPLANE as isize,
+ VideoOutputMplane = bindings::V4L2_CAP_VIDEO_OUTPUT_MPLANE as isize,
+ M2mMplane = bindings::V4L2_CAP_VIDEO_M2M_MPLANE as isize,
+ M2m = bindings::V4L2_CAP_VIDEO_M2M as isize,
+ Tuner = bindings::V4L2_CAP_TUNER as isize,
+ Audio = bindings::V4L2_CAP_AUDIO as isize,
+ Radio = bindings::V4L2_CAP_RADIO as isize,
+ Modulator = bindings::V4L2_CAP_MODULATOR as isize,
+ SdrCapture = bindings::V4L2_CAP_SDR_CAPTURE as isize,
+ ExtPixFormat = bindings::V4L2_CAP_EXT_PIX_FORMAT as isize,
+ SdrOutput = bindings::V4L2_CAP_SDR_OUTPUT as isize,
+ MetaCapture = bindings::V4L2_CAP_META_CAPTURE as isize,
+ ReadWrite = bindings::V4L2_CAP_READWRITE as isize,
+ Streaming = bindings::V4L2_CAP_STREAMING as isize,
+ MetaOutput = bindings::V4L2_CAP_META_OUTPUT as isize,
+ Touch = bindings::V4L2_CAP_TOUCH as isize,
+ IoMc = bindings::V4L2_CAP_IO_MC as isize,
+}
+
+/// A wrapper over a pointer to `struct v4l2_capability`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct CapabilitiesRef(*mut bindings::v4l2_capability);
+
+impl CapabilitiesRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`CapabilitiesRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_capability) -> Self {
+ Self(ptr)
+ }
+
+ // For internal convenience only.
+ fn as_mut(&mut self) -> &mut bindings::v4l2_capability {
+ // SAFETY: ptr is safe during the lifetime of [`CapabilitiesRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ /// Sets the `driver` field.
+ pub fn set_driver(&mut self, driver: &[u8]) {
+ let this = self.as_mut();
+ let len = core::cmp::min(driver.len(), this.driver.len());
+ this.driver[0..len].copy_from_slice(&driver[0..len]);
+ }
+
+ /// Sets the `card` field.
+ pub fn set_card(&mut self, card: &[u8]) {
+ let this = self.as_mut();
+ let len = core::cmp::min(card.len(), this.card.len());
+ this.card[0..len].copy_from_slice(&card[0..len]);
+ }
+
+ /// Sets the `bus_info` field.
+ pub fn set_bus_info(&mut self, bus_info: &[u8]) {
+ let this = self.as_mut();
+ let len = core::cmp::min(bus_info.len(), this.bus_info.len());
+ this.bus_info[0..len].copy_from_slice(&bus_info[0..len]);
+ }
+}
diff --git a/rust/kernel/media/v4l2/enums.rs b/rust/kernel/media/v4l2/enums.rs
new file mode 100644
index 000000000000..41397693c208
--- /dev/null
+++ b/rust/kernel/media/v4l2/enums.rs
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Enums
+//!
+//! Part of the following C header: [`include/linux/videodev2.h`](../../../../include/linux/videodev2.h)
+
+use crate::prelude::EINVAL;
+
+/// Buffer types as defined by `V4L2_BUF_TYPE_*`
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum BufType {
+ VideoCapture = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_CAPTURE as isize,
+ VideoOutput = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OUTPUT as isize,
+ VideoOverlay = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OVERLAY as isize,
+ VbiCapture = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VBI_CAPTURE as isize,
+ VbiOutput = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VBI_OUTPUT as isize,
+ SlicedVbiCapture = bindings::v4l2_buf_type_V4L2_BUF_TYPE_SLICED_VBI_CAPTURE as isize,
+ SlicedVbiOutput = bindings::v4l2_buf_type_V4L2_BUF_TYPE_SLICED_VBI_OUTPUT as isize,
+ VideoOutputOverlay = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY as isize,
+ VideoCaptureMplane = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE as isize,
+ VideoOutputMplane = bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE as isize,
+ SdrCapture = bindings::v4l2_buf_type_V4L2_BUF_TYPE_SDR_CAPTURE as isize,
+ SdrOutput = bindings::v4l2_buf_type_V4L2_BUF_TYPE_SDR_OUTPUT as isize,
+ MetaCapture = bindings::v4l2_buf_type_V4L2_BUF_TYPE_META_CAPTURE as isize,
+ MetaOutput = bindings::v4l2_buf_type_V4L2_BUF_TYPE_META_OUTPUT as isize,
+}
+
+impl TryFrom<u32> for BufType {
+ type Error = crate::error::Error;
+
+ fn try_from(value: u32) -> Result<Self, Self::Error> {
+ match value {
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_CAPTURE => Ok(Self::VideoCapture),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OUTPUT => Ok(Self::VideoOutput),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OVERLAY => Ok(Self::VideoOverlay),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VBI_CAPTURE => Ok(Self::VbiCapture),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VBI_OUTPUT => Ok(Self::VbiOutput),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_SLICED_VBI_CAPTURE => Ok(Self::SlicedVbiCapture),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_SLICED_VBI_OUTPUT => Ok(Self::SlicedVbiOutput),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY => {
+ Ok(Self::VideoOutputOverlay)
+ }
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE => {
+ Ok(Self::VideoCaptureMplane)
+ }
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE => {
+ Ok(Self::VideoOutputMplane)
+ }
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_SDR_CAPTURE => Ok(Self::SdrCapture),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_SDR_OUTPUT => Ok(Self::SdrOutput),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_META_CAPTURE => Ok(Self::MetaCapture),
+ bindings::v4l2_buf_type_V4L2_BUF_TYPE_META_OUTPUT => Ok(Self::MetaOutput),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
+/// Fields as defined by `V4L2_FIELD_*`
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum Field {
+ Any = bindings::v4l2_field_V4L2_FIELD_ANY as isize,
+ None = bindings::v4l2_field_V4L2_FIELD_NONE as isize,
+ Top = bindings::v4l2_field_V4L2_FIELD_TOP as isize,
+ Bottom = bindings::v4l2_field_V4L2_FIELD_BOTTOM as isize,
+ Interlaced = bindings::v4l2_field_V4L2_FIELD_INTERLACED as isize,
+ SeqTb = bindings::v4l2_field_V4L2_FIELD_SEQ_TB as isize,
+ SeqBt = bindings::v4l2_field_V4L2_FIELD_SEQ_BT as isize,
+ Alternate = bindings::v4l2_field_V4L2_FIELD_ALTERNATE as isize,
+ InterlacedTb = bindings::v4l2_field_V4L2_FIELD_INTERLACED_TB as isize,
+ InterlacedBt = bindings::v4l2_field_V4L2_FIELD_INTERLACED_BT as isize,
+}
+
+impl TryFrom<u32> for Field {
+ type Error = crate::error::Error;
+
+ fn try_from(value: u32) -> Result<Self, Self::Error> {
+ match value {
+ bindings::v4l2_field_V4L2_FIELD_ANY => Ok(Self::Any),
+ bindings::v4l2_field_V4L2_FIELD_NONE => Ok(Self::None),
+ bindings::v4l2_field_V4L2_FIELD_TOP => Ok(Self::Top),
+ bindings::v4l2_field_V4L2_FIELD_BOTTOM => Ok(Self::Bottom),
+ bindings::v4l2_field_V4L2_FIELD_INTERLACED => Ok(Self::Interlaced),
+ bindings::v4l2_field_V4L2_FIELD_SEQ_TB => Ok(Self::SeqTb),
+ bindings::v4l2_field_V4L2_FIELD_SEQ_BT => Ok(Self::SeqBt),
+ bindings::v4l2_field_V4L2_FIELD_ALTERNATE => Ok(Self::Alternate),
+ bindings::v4l2_field_V4L2_FIELD_INTERLACED_TB => Ok(Self::InterlacedTb),
+ bindings::v4l2_field_V4L2_FIELD_INTERLACED_BT => Ok(Self::InterlacedBt),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
+/// Colorspaces as defined by `V4L2_COLORSPACE_*`
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum Colorspace {
+ Default = bindings::v4l2_colorspace_V4L2_COLORSPACE_DEFAULT as isize,
+ Smpte170m = bindings::v4l2_colorspace_V4L2_COLORSPACE_SMPTE170M as isize,
+ Smpte240m = bindings::v4l2_colorspace_V4L2_COLORSPACE_SMPTE240M as isize,
+ Rec709 = bindings::v4l2_colorspace_V4L2_COLORSPACE_REC709 as isize,
+ Bt878 = bindings::v4l2_colorspace_V4L2_COLORSPACE_BT878 as isize,
+ SystemM470 = bindings::v4l2_colorspace_V4L2_COLORSPACE_470_SYSTEM_M as isize,
+ SystemBg470 = bindings::v4l2_colorspace_V4L2_COLORSPACE_470_SYSTEM_BG as isize,
+ Jpeg = bindings::v4l2_colorspace_V4L2_COLORSPACE_JPEG as isize,
+ Srgb = bindings::v4l2_colorspace_V4L2_COLORSPACE_SRGB as isize,
+ Oprgb = bindings::v4l2_colorspace_V4L2_COLORSPACE_OPRGB as isize,
+ Bt2020 = bindings::v4l2_colorspace_V4L2_COLORSPACE_BT2020 as isize,
+ Raw = bindings::v4l2_colorspace_V4L2_COLORSPACE_RAW as isize,
+ DciP3 = bindings::v4l2_colorspace_V4L2_COLORSPACE_DCI_P3 as isize,
+}
+
+impl TryFrom<u32> for Colorspace {
+ type Error = crate::error::Error;
+
+ fn try_from(value: u32) -> Result<Self, Self::Error> {
+ match value {
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_DEFAULT => Ok(Self::Default),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_SMPTE170M => Ok(Self::Smpte170m),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_SMPTE240M => Ok(Self::Smpte240m),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_REC709 => Ok(Self::Rec709),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_BT878 => Ok(Self::Bt878),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_470_SYSTEM_M => Ok(Self::SystemM470),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_470_SYSTEM_BG => Ok(Self::SystemBg470),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_JPEG => Ok(Self::Jpeg),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_SRGB => Ok(Self::Srgb),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_OPRGB => Ok(Self::Oprgb),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_BT2020 => Ok(Self::Bt2020),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_RAW => Ok(Self::Raw),
+ bindings::v4l2_colorspace_V4L2_COLORSPACE_DCI_P3 => Ok(Self::DciP3),
+ _ => Err(EINVAL),
+ }
+ }
+}
diff --git a/rust/kernel/media/v4l2/format.rs b/rust/kernel/media/v4l2/format.rs
new file mode 100644
index 000000000000..83bdd61b5c09
--- /dev/null
+++ b/rust/kernel/media/v4l2/format.rs
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Format Enumerations
+//!
+//! Part of the following C header: [`include/linux/videodev2.h`](../../../../include/linux/videodev2.h)
+
+use crate::media::v4l2::enums;
+use crate::prelude::*;
+
+/// A wrapper over a pointer to `struct v4l2_fmtdesc`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct FormatDescRef(*mut bindings::v4l2_fmtdesc);
+
+impl FormatDescRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`FormatDescRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_fmtdesc) -> Self {
+ Self(ptr)
+ }
+
+ fn as_mut(&mut self) -> &mut bindings::v4l2_fmtdesc {
+ // SAFETY: ptr is safe during the lifetime of [`FormatDescRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ fn as_ref(&self) -> &bindings::v4l2_fmtdesc {
+ // SAFETY: ptr is safe during the lifetime of [`FormatDescRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ /// Sets the `pixelformat` field.
+ pub fn set_pixel_format(&mut self, pixel_format: u32) {
+ let fmt = self.as_mut();
+ fmt.pixelformat = pixel_format;
+ }
+
+ /// Returns the `index` field.
+ pub fn index(&self) -> u32 {
+ self.as_ref().index
+ }
+
+ /// Returns the `type_` field.
+ pub fn type_(&self) -> u32 {
+ self.as_ref().type_
+ }
+}
+
+/// A wrapper over a pointer to `struct v4l2_format`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct FormatRef(*mut bindings::v4l2_format);
+
+impl FormatRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`FormatRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_format) -> Self {
+ Self(ptr)
+ }
+
+ // For internal convenience only.
+ fn as_mut(&mut self) -> &mut bindings::v4l2_format {
+ // SAFETY: ptr is safe during the lifetime of [`FormatDescRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ fn as_ref(&self) -> &bindings::v4l2_format {
+ // SAFETY: ptr is safe during the lifetime of [`FormatDescRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ /// Returns the `type_` field.
+ pub fn type_(&self) -> u32 {
+ self.as_ref().type_
+ }
+
+ /// Get the field `field` for the `pix` union member.
+ pub fn pix_field(&self) -> Result<enums::Field> {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ enums::Field::try_from(pix.field)
+ }
+
+ /// Get the field `width` for the `pix` union member.
+ pub fn pix_width(&self) -> u32 {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ pix.width
+ }
+
+ /// Get the field `height` for the `pix` union member.
+ pub fn pix_height(&self) -> u32 {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ pix.height
+ }
+
+ /// Get the field `pixelformat` for the `pix` union member.
+ pub fn pix_pixelformat(&self) -> u32 {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ pix.pixelformat
+ }
+
+ /// Get the field `bytesperline` for the `pix` union member.
+ pub fn pix_bytesperline(&self) -> u32 {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ pix.bytesperline
+ }
+
+ /// Get the field `sizeimage` for the `pix` union member.
+ pub fn pix_sizeimage(&self) -> u32 {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ pix.sizeimage
+ }
+
+ /// Get the field `colorspace` for the `pix` union member.
+ pub fn pix_colorspace(&self) -> Result<enums::Colorspace> {
+ let fmt = self.as_ref();
+ let pix = &unsafe { fmt.fmt.pix };
+ enums::Colorspace::try_from(pix.colorspace)
+ }
+
+ /// Set the field `field` for the `pix` union member.
+ pub fn set_pix_field(&mut self, field: enums::Field) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.field = field as u32;
+ }
+
+ /// Set the field `width` for the `pix` union member.
+ pub fn set_pix_width(&mut self, width: u32) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.width = width;
+ }
+
+ /// Set the field `height` for the `pix` union member.
+ pub fn set_pix_height(&mut self, height: u32) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.height = height;
+ }
+
+ /// Set the field `pixelformat` for the `pix` union member.
+ pub fn set_pix_pixel_format(&mut self, pixel_format: u32) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.pixelformat = pixel_format;
+ }
+
+ /// Set the field `bytesperline` for the `pix` union member.
+ pub fn set_pix_bytesperline(&mut self, bytesperline: u32) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.bytesperline = bytesperline;
+ }
+
+ /// Set the field `sizeimage` for the `pix` union member.
+ pub fn set_pix_sizeimage(&mut self, sizeimage: u32) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.sizeimage = sizeimage;
+ }
+
+ /// Set the field `sizeimage` for the `pix` union member.
+ pub fn set_pix_colorspace(&mut self, colorspace: enums::Colorspace) {
+ let fmt = self.as_mut();
+ let pix = &mut unsafe { fmt.fmt.pix };
+ pix.colorspace = colorspace as u32;
+ }
+}
diff --git a/rust/kernel/media/v4l2/framesize.rs b/rust/kernel/media/v4l2/framesize.rs
new file mode 100644
index 000000000000..2d015a7444be
--- /dev/null
+++ b/rust/kernel/media/v4l2/framesize.rs
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Frame size enumerations
+//!
+//! Part of the following C header: [`include/linux/videodev2.h`](../../../../include/linux/videodev2.h)
+
+/// Frame size types as defined in `V4L2_FRMSIZE_TYPE`
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+#[allow(missing_docs)]
+pub enum FrameSizeType {
+ Discrete = bindings::v4l2_frmsizetypes_V4L2_FRMSIZE_TYPE_DISCRETE as isize,
+ Continuous = bindings::v4l2_frmsizetypes_V4L2_FRMSIZE_TYPE_CONTINUOUS as isize,
+ Stepwise = bindings::v4l2_frmsizetypes_V4L2_FRMSIZE_TYPE_STEPWISE as isize,
+}
+
+impl TryFrom<u32> for FrameSizeType {
+ type Error = crate::error::Error;
+
+ fn try_from(value: u32) -> Result<Self, Self::Error> {
+ match value {
+ bindings::v4l2_frmsizetypes_V4L2_FRMSIZE_TYPE_DISCRETE => Ok(Self::Discrete),
+ bindings::v4l2_frmsizetypes_V4L2_FRMSIZE_TYPE_CONTINUOUS => Ok(Self::Continuous),
+ bindings::v4l2_frmsizetypes_V4L2_FRMSIZE_TYPE_STEPWISE => Ok(Self::Stepwise),
+ _ => Err(crate::prelude::EINVAL),
+ }
+ }
+}
+
+/// A wrapper over a pointer to `struct v4l2_frmsizeenum`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct FrameSizeRef(*mut bindings::v4l2_frmsizeenum);
+
+impl FrameSizeRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`FrameSizeRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_frmsizeenum) -> Self {
+ Self(ptr)
+ }
+
+ fn as_ref(&self) -> &bindings::v4l2_frmsizeenum {
+ // SAFETY: ptr is safe during the lifetime of [`FrameSizeRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_ref().unwrap() }
+ }
+
+ fn as_mut(&mut self) -> &mut bindings::v4l2_frmsizeenum {
+ // SAFETY: ptr is safe during the lifetime of [`FrameSizeRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ /// Sets the `index` member.
+ pub fn set_index(&mut self, index: u32) {
+ let this = self.as_mut();
+ this.index = index;
+ }
+
+ /// Sets the `pixel_format` member.
+ pub fn set_pixel_format(&mut self, pixel_format: u32) {
+ let this = self.as_mut();
+ this.pixel_format = pixel_format;
+ }
+
+ /// Sets the `type_` member.
+ pub fn set_type(&mut self, type_: FrameSizeType) {
+ let this = self.as_mut();
+ this.type_ = type_ as _;
+ }
+
+ /// Sets the `width` member from the `discrete` union member.
+ pub fn set_discrete_width(&mut self, width: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.discrete.width = width;
+ }
+
+ /// Sets the `height` member from the `discrete` union member.
+ pub fn set_discrete_height(&mut self, height: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.discrete.height = height;
+ }
+
+ /// Sets the `min_width` member from the `step_wise` union member.
+ pub fn set_stepwise_min_width(&mut self, min_width: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.stepwise.min_width = min_width;
+ }
+
+ /// Sets the `max_width` member from the `step_wise` union member.
+ pub fn set_stepwise_max_width(&mut self, max_width: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.stepwise.max_width = max_width;
+ }
+
+ /// Sets the `step_width` member from the `step_wise` union member.
+ pub fn set_stepwise_step_width(&mut self, step_width: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.stepwise.step_width = step_width;
+ }
+
+ /// Sets the `min_height` member from the `step_wise` union member.
+ pub fn set_stepwise_min_height(&mut self, min_height: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.stepwise.min_height = min_height;
+ }
+
+ /// Sets the `max_height` member from the `step_wise` union member.
+ pub fn set_stepwise_max_height(&mut self, max_height: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.stepwise.max_height = max_height;
+ }
+
+ /// Sets the `step_height` member from the `step_wise` union member.
+ pub fn set_stepwise_step_height(&mut self, step_height: u32) {
+ let this = self.as_mut();
+ this.__bindgen_anon_1.stepwise.step_height = step_height;
+ }
+
+ /// Returns the `index` member.
+ pub fn index(&self) -> u32 {
+ self.as_ref().index
+ }
+
+ /// Returns the `pixel_format` member.
+ pub fn pixel_format(&self) -> u32 {
+ self.as_ref().pixel_format
+ }
+
+ /// Returns the `width` member from the `discrete` union member.
+ pub fn discrete_width(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.discrete.width }
+ }
+
+ /// Returns the `height` member from the `discrete` union member.
+ pub fn discrete_height(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.discrete.height }
+ }
+
+ /// Returns the `min_width` member from the `step_wise` union member.
+ pub fn stepwise_min_width(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.stepwise.min_width }
+ }
+
+ /// Returns the `max_width` member from the `step_wise` union member.
+ pub fn stepwise_max_width(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.stepwise.max_width }
+ }
+
+ /// Returns the `step_width` member from the `step_wise` union member.
+ pub fn stepwise_step_width(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.stepwise.step_width }
+ }
+
+ /// Returns the `min_height` member from the `step_wise` union member.
+ pub fn stepwise_min_height(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.stepwise.min_height }
+ }
+
+ /// Returns the `max_height` member from the `step_wise` union member.
+ pub fn stepwise_max_height(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.stepwise.max_height }
+ }
+
+ /// Returns the `step_height` member from the `step_wise` union member.
+ pub fn stepwise_step_height(&self) -> u32 {
+ let this = self.as_ref();
+ unsafe { this.__bindgen_anon_1.stepwise.step_height }
+ }
+}
diff --git a/rust/kernel/media/v4l2/inputs.rs b/rust/kernel/media/v4l2/inputs.rs
new file mode 100644
index 000000000000..8361971e2983
--- /dev/null
+++ b/rust/kernel/media/v4l2/inputs.rs
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Video Inputs
+//!
+//! Part of the following C header: [`include/linux/videodev2.h`](../../../../include/linux/videodev2.h)
+
+use crate::prelude::*;
+use crate::str::CStr;
+
+/// A wrapper over a pointer to `struct v4l2_input`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct InputRef(*mut bindings::v4l2_input);
+
+impl InputRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`InputRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_input) -> Self {
+ Self(ptr)
+ }
+
+ fn as_mut(&mut self) -> &mut bindings::v4l2_input {
+ // SAFETY: ptr is safe during the lifetime of [`InputRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_mut().unwrap() }
+ }
+
+ fn as_ref(&self) -> &bindings::v4l2_input {
+ // SAFETY: ptr is safe during the lifetime of [`InputRef`] as per
+ // the safety requirement in `from_ptr()`
+ unsafe { self.0.as_ref().unwrap() }
+ }
+
+ /// Returns the `index` member.
+ pub fn index(&self) -> u32 {
+ let this = self.as_ref();
+ this.index
+ }
+
+ /// Returns the `name` member.
+ pub fn name(&self) -> core::result::Result<&CStr, crate::str::CStrConvertError> {
+ let this = self.as_ref();
+ CStr::from_bytes_with_nul(&this.name)
+ }
+
+ /// Returns the `type_` member.
+ pub fn type_(&self) -> Result<Type> {
+ let this = self.as_ref();
+ Type::try_from(this.type_)
+ }
+
+ /// Returns the `status` member.
+ pub fn status(&self) -> u32 {
+ let this = self.as_ref();
+ this.status
+ }
+
+ /// Sets the `name` member.
+ pub fn set_name(&mut self, name: &CStr) {
+ let this = self.as_mut();
+ let len = core::cmp::min(name.len(), this.name.len());
+ this.name[0..len].copy_from_slice(&name[0..len]);
+ }
+
+ /// Sets the `type_` member.
+ pub fn set_type(&mut self, type_: Type) {
+ let this = self.as_mut();
+ this.type_ = type_ as u32;
+ }
+
+ /// Sets the `std` member.
+ pub fn set_std(&mut self, std: u64) {
+ let this = self.as_mut();
+ this.std = std;
+ }
+
+ /// Sets the `status` member.
+ pub fn set_status(&mut self, status: u32) {
+ let this = self.as_mut();
+ this.status = status;
+ }
+}
+
+/// Input types as defined by `V4L2_INPUT_TYPE_*`
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum Type {
+ Tuner = bindings::V4L2_INPUT_TYPE_TUNER as isize,
+ Camera = bindings::V4L2_INPUT_TYPE_CAMERA as isize,
+ Touch = bindings::V4L2_INPUT_TYPE_TOUCH as isize,
+}
+
+impl TryFrom<u32> for Type {
+ type Error = crate::error::Error;
+
+ fn try_from(value: u32) -> Result<Self> {
+ match value {
+ bindings::V4L2_INPUT_TYPE_TUNER => Ok(Self::Tuner),
+ bindings::V4L2_INPUT_TYPE_CAMERA => Ok(Self::Camera),
+ bindings::V4L2_INPUT_TYPE_TOUCH => Ok(Self::Touch),
+ _ => Err(EINVAL),
+ }
+ }
+}
diff --git a/rust/kernel/media/v4l2/mmap.rs b/rust/kernel/media/v4l2/mmap.rs
new file mode 100644
index 000000000000..2d9ce2ceb148
--- /dev/null
+++ b/rust/kernel/media/v4l2/mmap.rs
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Memory-mapping buffers
+//!
+//! Part of the following C header: [`include/linux/videodev2.h`](../../../../include/linux/videodev2.h)
+
+/// Buffer flags as defined by `V4L2_BUF_FLAG_*`.
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum BufferFlag {
+ Mapped = bindings::V4L2_BUF_FLAG_MAPPED as isize,
+ Queued = bindings::V4L2_BUF_FLAG_QUEUED as isize,
+ Done = bindings::V4L2_BUF_FLAG_DONE as isize,
+ Keyframe = bindings::V4L2_BUF_FLAG_KEYFRAME as isize,
+ Pframe = bindings::V4L2_BUF_FLAG_PFRAME as isize,
+ Bframe = bindings::V4L2_BUF_FLAG_BFRAME as isize,
+ Error = bindings::V4L2_BUF_FLAG_ERROR as isize,
+ InRequest = bindings::V4L2_BUF_FLAG_IN_REQUEST as isize,
+ Timecode = bindings::V4L2_BUF_FLAG_TIMECODE as isize,
+ HoldCaptureBuf = bindings::V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF as isize,
+ Prepared = bindings::V4L2_BUF_FLAG_PREPARED as isize,
+ NoCacheInvalidate = bindings::V4L2_BUF_FLAG_NO_CACHE_INVALIDATE as isize,
+ NoCacheClean = bindings::V4L2_BUF_FLAG_NO_CACHE_CLEAN as isize,
+ TimestampMonotonic = bindings::V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC as isize,
+ TimestampCopy = bindings::V4L2_BUF_FLAG_TIMESTAMP_COPY as isize,
+ TstampSrcSoe = bindings::V4L2_BUF_FLAG_TSTAMP_SRC_SOE as isize,
+ Last = bindings::V4L2_BUF_FLAG_LAST as isize,
+ RequestFd = bindings::V4L2_BUF_FLAG_REQUEST_FD as isize,
+}
+
+/// A wrapper over a pointer to `struct v4l2_exportbuffer`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct ExportBufferRef(*mut bindings::v4l2_exportbuffer);
+
+impl ExportBufferRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`ExportBufferRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_exportbuffer) -> Self {
+ Self(ptr)
+ }
+}
+
+/// A wrapper over a pointer to `struct v4l2_requestbuffers`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct RequestBuffersRef(*mut bindings::v4l2_requestbuffers);
+
+impl RequestBuffersRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`RequestBuffersRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_requestbuffers) -> Self {
+ Self(ptr)
+ }
+}
+
+/// A wrapper over a pointer to `struct v4l2_create_buffers`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct CreateBuffersRef(*mut bindings::v4l2_create_buffers);
+
+impl CreateBuffersRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`CreateBuffersRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_create_buffers) -> Self {
+ Self(ptr)
+ }
+}
+
+/// A wrapper over a pointer to `struct v4l2_buffer`.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct BufferRef(*mut bindings::v4l2_buffer);
+
+impl BufferRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`BufferRef`] instance.
+ pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_buffer) -> Self {
+ Self(ptr)
+ }
+}
diff --git a/rust/kernel/media/v4l2/mod.rs b/rust/kernel/media/v4l2/mod.rs
index 068dd9b4863d..77864640f19e 100644
--- a/rust/kernel/media/v4l2/mod.rs
+++ b/rust/kernel/media/v4l2/mod.rs
@@ -1,3 +1,10 @@
// SPDX-License-Identifier: GPL-2.0 OR MIT

//! Abstractions for include/media/v4l2-*.h
+
+pub mod capabilities;
+pub mod enums;
+pub mod format;
+pub mod framesize;
+pub mod inputs;
+pub mod mmap;
--
2.40.0

2023-04-06 21:59:18

[permalink] [raw]

Subject: [PATCH 4/6] rust: media: videobuf2: add a videobuf2 abstraction

Add a videobuf2 abstraction. Notably, only vb2_v4l2_buffer is supported
as the subsystem-specific struct.

Signed-off-by: Daniel Almeida <[email protected]>
---
rust/bindings/bindings_helper.h | 3 +
rust/kernel/media/mod.rs | 1 +
rust/kernel/media/videobuf2/core.rs | 552 ++++++++++++++++++++++++++++
rust/kernel/media/videobuf2/mod.rs | 5 +
4 files changed, 561 insertions(+)
create mode 100644 rust/kernel/media/videobuf2/core.rs
create mode 100644 rust/kernel/media/videobuf2/mod.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3b3d6fcf110f..3153894f426b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -36,6 +36,9 @@
#include <linux/uaccess.h>
#include <linux/uio.h>
#include <linux/videodev2.h>
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-v4l2.h>
+#include <media/videobuf2-dma-sg.h>

/* `bindgen` gets confused at certain things. */
const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/media/mod.rs b/rust/kernel/media/mod.rs
index 342e66382719..95b11544af8d 100644
--- a/rust/kernel/media/mod.rs
+++ b/rust/kernel/media/mod.rs
@@ -3,3 +3,4 @@
//! Media subsystem

pub mod v4l2;
+pub mod videobuf2;
diff --git a/rust/kernel/media/videobuf2/core.rs b/rust/kernel/media/videobuf2/core.rs
new file mode 100644
index 000000000000..40ab06475a8e
--- /dev/null
+++ b/rust/kernel/media/videobuf2/core.rs
@@ -0,0 +1,552 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Video Buffer 2 Core Framework
+//!
+//! C header: [`include/media/videobuf2-core.h`](../../../../include/media/videobuf2-core.h)
+
+use core::cell::UnsafeCell;
+use core::marker::PhantomData;
+use core::ops::Deref;
+
+use alloc::slice;
+
+use crate::device::RawDevice;
+use crate::error::from_kernel_result;
+use crate::media::v4l2::{enums, mmap};
+use crate::prelude::*;
+use crate::sync::ffi_mutex::FfiMutex;
+use crate::sync::Arc;
+use crate::ForeignOwnable;
+
+/// The queue operations to be implemented by drivers.
+#[vtable]
+pub trait QueueOperations {
+ /// The subsystem-specific data, usually bindings::vb2_v4l2_buffer.
+ type SubsystemSpecificData = bindings::vb2_v4l2_buffer;
+ /// The driver specific data.
+ type DriverSpecificData;
+ /// The private data held in the queue.
+ type PrivateData: ForeignOwnable
+ + Deref<Target = PrivateData<Self::SubsystemSpecificData, Self::DriverSpecificData>>;
+
+ // Note: alloc_devs is TODO
+
+ /// Called as part of the queue_setup() queue operation.
+ fn queue_setup(
+ queue: &QueueRef,
+ private_data: &mut Self::PrivateData,
+ num_buffers: &mut u32,
+ num_planes: &mut u32,
+ sizes: &mut [u32],
+ ) -> Result;
+
+ /// Called as part of the wait_prepare() queue operation.
+ fn wait_prepare(_queue: &QueueRef, _private_data: &mut Self::PrivateData) {}
+
+ /// Called as part of the wait_finish() queue operation.
+ fn wait_finish(_queue: &QueueRef, _private_data: &mut Self::PrivateData) {}
+
+ /// Optional. Called as part of the buf_out_validate() queue operation.
+ fn buf_out_validate(_buffer: &Buffer, _private_data: &mut Self::PrivateData) -> Result {
+ Ok(())
+ }
+
+ /// Called as part of the buf_init() queue operation.
+ fn buf_init(_buffer: &Buffer, _private_data: &mut Self::PrivateData) -> Result;
+ /// Called as part of the buf_prepare() queue operation.
+ fn buf_prepare(_buffer: &Buffer, _private_data: &mut Self::PrivateData) -> Result;
+ /// Called as part of the buf_finish() queue operation.
+ fn buf_finish(_buffer: &Buffer, _private_data: &mut Self::PrivateData) {}
+ /// Called as part of the buf_cleanup() queue operation.
+ fn buf_cleanup(_buffer: &Buffer, _private_data: &mut Self::PrivateData);
+
+ /// Called as part of the prepare_streaming() queue operation.
+ fn prepare_streaming(_queue: &QueueRef, _private_data: &mut Self::PrivateData) -> Result {
+ Ok(())
+ }
+ /// Called as part of the start_streaming() queue operation.
+ fn start_streaming(
+ _queue: &QueueRef,
+ _private_data: &mut Self::PrivateData,
+ _count: u32,
+ ) -> Result;
+
+ /// Called as part of the unprepare_streaming() queue operation.
+ fn unprepare_streaming(_queue: &QueueRef, _private_data: &mut Self::PrivateData) {}
+ /// Called as part of the stop_streaming() queue operation.
+ fn stop_streaming(_queue: &QueueRef, _private_data: &mut Self::PrivateData);
+ /// Called as part of the buf_queue() queue operation.
+ fn buf_queue(_buffer: &Buffer, _private_data: &mut Self::PrivateData);
+ /// Called as part of the buf_request_complete() queue operation.
+ fn buf_request_complete(_buffer: &Buffer, _private_data: &mut Self::PrivateData) {}
+}
+
+type AsPrivateData<T> = <T as QueueOperations>::PrivateData;
+
+struct Vb2OperationsVtable<T: QueueOperations>(PhantomData<T>);
+
+impl<T: QueueOperations> Vb2OperationsVtable<T> {
+ unsafe extern "C" fn queue_setup_callback(
+ q: *mut bindings::vb2_queue,
+ num_buffers: *mut core::ffi::c_uint,
+ num_planes: *mut core::ffi::c_uint,
+ sizes: *mut core::ffi::c_uint,
+ _alloc_devs: *mut *mut bindings::device,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ // SAFETY: `q` is a pointer returned by the kernel and outlives the
+ // QueueRef as QueueRef is dropped by the end of this function
+ let queue = unsafe { QueueRef::from_ptr(q) };
+
+ // SAFETY: into_foreign() was called in Queue::new() and this
+ // function is called because we set the ops during Queue::new().
+ // Furthermore, from_pointer() is only called when Queue is dropped,
+ // at which point we cannot be called anymore because we will have
+ // called vb2_queue_release. There are no other concurrent uses of
+ // the pointer until the guard is dropped.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+
+ // SAFETY: sizes is known to be of size bindings::VB2_MAX_PLANES as
+ // per videobuf2-core.c::vb2_core_reqbufs()
+ let sizes = unsafe { slice::from_raw_parts_mut(sizes as _, bindings::VB2_MAX_PLANES as usize) };
+
+ // SAFETY: these are both safe, since they're initialized in
+ // videobuf2-core.c::vb2_core_reqbufs() and they are not read from any
+ // other pointer while the reference exists, since they live in the
+ // stack of vb2_core_reqbufs().
+ let num_buffers = unsafe { num_buffers.as_mut().ok_or(EINVAL) }?;
+ let num_planes = unsafe { num_planes.as_mut().ok_or(EINVAL) }?;
+
+ T::queue_setup(&queue, &mut private_data, num_buffers, num_planes, sizes)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn wait_prepare_callback(q: *mut bindings::vb2_queue) {
+ // SAFETY: QueueRef has a shorter lifetime if compared to the lifetime
+ // of q. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let queue = unsafe { QueueRef::from_ptr(q) };
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+ T::wait_prepare(&queue, &mut private_data);
+ }
+
+ unsafe extern "C" fn wait_finish_callback(q: *mut bindings::vb2_queue) {
+ // Same safety comment from wait_prepare_callback applies.
+ let queue = unsafe { QueueRef::from_ptr(q) };
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+ T::wait_finish(&queue, &mut private_data);
+ }
+
+ unsafe extern "C" fn buf_out_validate_callback(
+ vb: *mut bindings::vb2_buffer,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_out_validate(&buf, &mut private_data)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn buf_init_callback(vb: *mut bindings::vb2_buffer) -> core::ffi::c_int {
+ from_kernel_result! {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_init(&buf, &mut private_data)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn buf_prepare_callback(vb: *mut bindings::vb2_buffer) -> core::ffi::c_int {
+ from_kernel_result! {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_prepare(&buf, &mut private_data)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn buf_finish_callback(vb: *mut bindings::vb2_buffer) {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_finish(&buf, &mut private_data);
+ }
+
+ unsafe extern "C" fn buf_cleanup_callback(vb: *mut bindings::vb2_buffer) {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_cleanup(&buf, &mut private_data);
+ }
+
+ unsafe extern "C" fn prepare_streaming_callback(
+ q: *mut bindings::vb2_queue,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ // SAFETY: QueueRef has a shorter lifetime if compared to the lifetime
+ // of q. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let queue = unsafe { QueueRef::from_ptr(q) };
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+ T::prepare_streaming(&queue, &mut private_data)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn start_streaming_callback(
+ q: *mut bindings::vb2_queue,
+ count: core::ffi::c_uint,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ // Same safety comments from queue_setup() apply.
+ let queue = unsafe { QueueRef::from_ptr(q) };
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+ T::start_streaming(&queue, &mut private_data, count)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn stop_streaming_callback(q: *mut bindings::vb2_queue) {
+ // Same safety comments from queue_setup() apply.
+ let queue = unsafe { QueueRef::from_ptr(q) };
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+ T::stop_streaming(&queue, &mut private_data);
+ }
+
+ unsafe extern "C" fn unprepare_streaming_callback(q: *mut bindings::vb2_queue) {
+ // SAFETY: QueueRef has a shorter lifetime if compared to the lifetime
+ // of q. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let queue = unsafe { QueueRef::from_ptr(q) };
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*q).drv_priv) };
+ T::unprepare_streaming(&queue, &mut private_data);
+ }
+
+ unsafe extern "C" fn buf_queue_callback(vb: *mut bindings::vb2_buffer) {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_queue(&buf, &mut private_data);
+ }
+
+ unsafe extern "C" fn buf_request_complete_callback(vb: *mut bindings::vb2_buffer) {
+ // SAFETY: Buffer has a shorter lifetime if compared to the lifetime
+ // of vb. The pointer is passed in by the kernel and thus it is assumed
+ // valid.
+ let buf = unsafe { Buffer::from_ptr(vb) };
+ let queue_ptr = buf.queue_ptr();
+
+ // Same safety comments from queue_setup_callback apply.
+ let mut private_data = unsafe { T::PrivateData::borrow_mut((*queue_ptr).drv_priv) };
+
+ T::buf_request_complete(&buf, &mut private_data);
+ }
+
+ const VTABLE: bindings::vb2_ops = bindings::vb2_ops {
+ queue_setup: Some(Self::queue_setup_callback),
+ wait_prepare: if T::HAS_WAIT_PREPARE {
+ Some(Self::wait_prepare_callback)
+ } else {
+ None
+ },
+ wait_finish: if T::HAS_WAIT_FINISH {
+ Some(Self::wait_finish_callback)
+ } else {
+ None
+ },
+ buf_out_validate: if T::HAS_BUF_OUT_VALIDATE {
+ Some(Self::buf_out_validate_callback)
+ } else {
+ None
+ },
+ buf_init: Some(Self::buf_init_callback),
+ buf_prepare: Some(Self::buf_prepare_callback),
+ buf_finish: if T::HAS_BUF_FINISH {
+ Some(Self::buf_finish_callback)
+ } else {
+ None
+ },
+ buf_cleanup: Some(Self::buf_cleanup_callback),
+ start_streaming: Some(Self::start_streaming_callback),
+ stop_streaming: Some(Self::stop_streaming_callback),
+ buf_queue: Some(Self::buf_queue_callback),
+ buf_request_complete: if T::HAS_BUF_REQUEST_COMPLETE {
+ Some(Self::buf_request_complete_callback)
+ } else {
+ None
+ },
+ prepare_streaming: if T::HAS_PREPARE_STREAMING {
+ Some(Self::prepare_streaming_callback)
+ } else {
+ None
+ },
+ unprepare_streaming: if T::HAS_UNPREPARE_STREAMING {
+ Some(Self::unprepare_streaming_callback)
+ } else {
+ None
+ },
+ };
+
+ fn build() -> &'static bindings::vb2_ops {
+ &Self::VTABLE
+ }
+}
+
+/// The IO modes as defined by `struct vb2_io_mode::VB2_*`.
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum IoModes {
+ Mmap = bindings::vb2_io_modes_VB2_MMAP as isize,
+ UserPtr = bindings::vb2_io_modes_VB2_USERPTR as isize,
+ Read = bindings::vb2_io_modes_VB2_READ as isize,
+ Write = bindings::vb2_io_modes_VB2_WRITE as isize,
+ DmaBuf = bindings::vb2_io_modes_VB2_DMABUF as isize,
+}
+
+/// Controls the value assigned to the `mem_ops` member.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub enum MemOps {
+ /// Sets `bindings::vb2_dma_sg_memops`.
+ DmaSg,
+}
+
+/// Represents the driver-specific buffer structure. This must contain the
+/// subsystem-specific structure as its first member. For now, only `struct
+/// vb2_v4l2_buffer` is supported as the subsystem-specific structure.
+#[repr(C)]
+pub struct PrivateData<T, U> {
+ subsystem_specific: T,
+ driver_specific: U,
+}
+
+impl<T, U> PrivateData<T, U> {
+ /// Returns the size of the driver-specific struct.
+ pub fn driver_data_size(&self) -> usize {
+ core::mem::size_of_val(&self.driver_specific)
+ }
+
+ /// Returns the subsystem-specific struct.
+ pub fn subsystem_specific(&self) -> &T {
+ &self.subsystem_specific
+ }
+
+ /// Returns the driver-specific struct.
+ pub fn driver_specific_mut(&mut self) -> &mut U {
+ &mut self.driver_specific
+ }
+}
+
+impl<U> PrivateData<bindings::vb2_v4l2_buffer, U> {
+ /// Creates a new instance.
+ pub fn new(data: U) -> Self {
+ Self {
+ subsystem_specific: Default::default(),
+ driver_specific: data,
+ }
+ }
+}
+
+/// A struct containing the parameters for queue creation.
+#[allow(missing_docs)]
+pub struct QueueCreateInfo<'a, T: QueueOperations, U: RawDevice> {
+ pub buf_type: enums::BufType,
+ pub io_modes: &'a [IoModes],
+ pub dev: Arc<U>,
+ pub timestamp_flags: &'a [mmap::BufferFlag],
+ pub lock: Option<Pin<Box<FfiMutex>>>,
+ pub min_buffers_needed: usize,
+ pub gfp_flags: u32,
+ pub mem_ops: MemOps,
+ pub requires_requests: bool,
+ pub supports_requests: bool,
+ pub private_data: Option<T::PrivateData>,
+}
+
+/// An owned wrapper over `struct vb2_queue`.
+pub struct Queue<T: QueueOperations> {
+ inner: UnsafeCell<bindings::vb2_queue>,
+ has_private_data: bool,
+ _lock: Option<Pin<Box<FfiMutex>>>,
+ _p1: PhantomData<T>,
+}
+
+impl<T: QueueOperations> Queue<T> {
+ /// Creates a new queue.
+ pub fn new<U: RawDevice>(create_info: QueueCreateInfo<'_, T, U>) -> Result<Self> {
+ let QueueCreateInfo {
+ buf_type,
+ io_modes,
+ dev,
+ timestamp_flags,
+ mut lock,
+ min_buffers_needed,
+ gfp_flags,
+ mem_ops,
+ requires_requests,
+ supports_requests,
+ private_data,
+ } = create_info;
+
+ let mut io_modes_val = 0;
+ for io_mode in io_modes {
+ io_modes_val |= *io_mode as u32;
+ }
+
+ let mut timestamp_flags_val = 0;
+ for timestamp_flag in timestamp_flags {
+ timestamp_flags_val |= *timestamp_flag as u32;
+ }
+
+ let (buf_struct_size, drv_priv, has_private_data) = if let Some(private_data) = private_data
+ {
+ (
+ private_data.driver_data_size(),
+ private_data.into_foreign() as *mut _,
+ true,
+ )
+ } else {
+ (0, core::ptr::null_mut(), false)
+ };
+
+ let mut inner = bindings::vb2_queue {
+ type_: buf_type as _,
+ io_modes: io_modes_val,
+ dev: dev.raw_device(),
+ ..Default::default()
+ };
+
+ if let Some(ref mut lock) = lock {
+ let raw_lock = unsafe { lock.as_mut().raw() };
+ inner.lock = raw_lock;
+ }
+
+ inner.drv_priv = drv_priv;
+ inner.buf_struct_size = buf_struct_size as u32;
+ inner.timestamp_flags = timestamp_flags_val;
+ inner.gfp_flags = gfp_flags;
+ inner.min_buffers_needed = min_buffers_needed as u32;
+
+ inner.set_requires_requests(requires_requests as u32);
+ inner.set_supports_requests(supports_requests as u32);
+
+ inner.ops = Vb2OperationsVtable::<T>::build();
+
+ inner.mem_ops = match mem_ops {
+ // SAFETY: this will be called by the C code.
+ MemOps::DmaSg => unsafe { &bindings::vb2_dma_sg_memops as _ },
+ };
+
+ // SAFETY: just a FFI call and we have just initialized `inner`.
+ unsafe {
+ bindings::vb2_queue_init(&mut inner as _);
+ }
+
+ let inner = UnsafeCell::new(inner);
+
+ Ok(Self {
+ inner,
+ has_private_data,
+ _p1: PhantomData,
+ _lock: lock,
+ })
+ }
+}
+
+impl<T: QueueOperations> Drop for Queue<T> {
+ fn drop(&mut self) {
+ if self.has_private_data {
+ // SAFETY: into_pointer() was previously called during new() and
+ // `self.has_private_data` was set accordingly.
+ unsafe {
+ AsPrivateData::<T>::from_foreign(self.inner.get_mut().drv_priv);
+ }
+ }
+
+ // SAFETY: just a FFI call and we have initialized `inner` during new().
+ unsafe {
+ bindings::vb2_queue_release(self.inner.get());
+ }
+ }
+}
+
+// SAFETY: This is a wrapper over the `struct vb2_queue` C type, which can be
+// used from any thread.
+unsafe impl<T: QueueOperations> Send for Queue<T> {}
+
+/// A wrapper over a pointer to `struct vb2_queue`.
+pub struct QueueRef {
+ _ptr: *mut bindings::vb2_queue,
+}
+
+impl QueueRef {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`QueueRef`] instance.
+ unsafe fn from_ptr(ptr: *mut bindings::vb2_queue) -> Self {
+ Self { _ptr: ptr }
+ }
+}
+
+/// A wrapper over a pointer to `struct vb2_buffer`.
+pub struct Buffer {
+ ptr: *mut bindings::vb2_buffer,
+}
+
+impl Buffer {
+ /// # Safety
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`Buffer`] instance.
+ pub(crate) unsafe fn from_ptr(ptr: *mut bindings::vb2_buffer) -> Self {
+ Self { ptr }
+ }
+
+ pub(crate) fn queue_ptr(&self) -> *mut bindings::vb2_queue {
+ // SAFETY: self.ptr should be valid according to the safety requirements in `from_raw()`.
+ unsafe { (*self.ptr).vb2_queue }
+ }
+}
diff --git a/rust/kernel/media/videobuf2/mod.rs b/rust/kernel/media/videobuf2/mod.rs
new file mode 100644
index 000000000000..6b0be2c7a14d
--- /dev/null
+++ b/rust/kernel/media/videobuf2/mod.rs
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Videobuf2 Framework
+
+pub mod core;
--
2.40.0

2023-04-06 22:00:55

[permalink] [raw]

Subject: [PATCH 5/6] rust: media: add {video|v4l2}_device_register support

Add {video|v4l2}_device_register support, essentially introducing
(initial) support for v4l2 drivers in Rust.

Signed-off-by: Daniel Almeida <[email protected]>
---
rust/bindings/bindings_helper.h | 4 +
rust/kernel/media/v4l2/dev.rs | 369 +++++++++++++++++++
rust/kernel/media/v4l2/device.rs | 115 ++++++
rust/kernel/media/v4l2/ioctls.rs | 608 +++++++++++++++++++++++++++++++
rust/kernel/media/v4l2/mod.rs | 3 +
5 files changed, 1099 insertions(+)
create mode 100644 rust/kernel/media/v4l2/dev.rs
create mode 100644 rust/kernel/media/v4l2/device.rs
create mode 100644 rust/kernel/media/v4l2/ioctls.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3153894f426b..bf2e833e511c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -36,6 +36,10 @@
#include <linux/uaccess.h>
#include <linux/uio.h>
#include <linux/videodev2.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-ioctl.h>
#include <media/videobuf2-core.h>
#include <media/videobuf2-v4l2.h>
#include <media/videobuf2-dma-sg.h>
diff --git a/rust/kernel/media/v4l2/dev.rs b/rust/kernel/media/v4l2/dev.rs
new file mode 100644
index 000000000000..32a7573f3439
--- /dev/null
+++ b/rust/kernel/media/v4l2/dev.rs
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! V4L2 Driver Helper API
+//!
+//! C header: [`include/media/v4l2-dev.h`](../../../../include/media/v4l2-dev.h)
+
+use core::marker::PhantomData;
+use core::pin::Pin;
+
+use crate::device;
+use crate::media::v4l2::capabilities;
+use crate::media::v4l2::device::V4l2Device;
+use crate::prelude::*;
+use crate::sync::smutex::Mutex;
+use crate::sync::Arc;
+use crate::ForeignOwnable;
+use crate::Result;
+use crate::ThisModule;
+
+/// The driver trait to be implemented by driver authors.
+pub trait Driver: crate::media::v4l2::ioctls::Operations<PrivateData = Self::DriverData> {
+ /// The driver's private data.
+ type DriverData: ForeignOwnable + Sync + Send;
+}
+
+/// VflDevNodeTypes as described by `VFL_TYPE_*` constants.
+#[allow(missing_docs)]
+pub enum VflDevNodeType {
+ Video = bindings::vfl_devnode_type_VFL_TYPE_VIDEO as isize,
+ Vbi = bindings::vfl_devnode_type_VFL_TYPE_VBI as isize,
+ Radio = bindings::vfl_devnode_type_VFL_TYPE_RADIO as isize,
+ Subdev = bindings::vfl_devnode_type_VFL_TYPE_SUBDEV as isize,
+ Sdr = bindings::vfl_devnode_type_VFL_TYPE_SDR as isize,
+ Touch = bindings::vfl_devnode_type_VFL_TYPE_TOUCH as isize,
+}
+
+/// A Video Device.
+pub struct VideoDevice<T> {
+ /// A raw pointer to `struct video_device` in C.
+ pub(crate) device: bindings::video_device,
+ /// A phantom for T. We need T to access the associated constants set by
+ /// drivers.
+ pub(crate) _phantom: PhantomData<T>,
+ /// Associated V4l2Device that must be kept alive while the VideoDevice is
+ /// in use.
+ _v4l2_device: Arc<Mutex<V4l2Device>>,
+}
+
+impl<T> VideoDevice<T> {
+ fn new(v4l2_device: Arc<Mutex<V4l2Device>>) -> Result<Self> {
+ let mut device = bindings::video_device::default();
+
+ let mut guard = v4l2_device.as_ref().lock();
+ let inner = &mut *guard;
+ let v4l2_device_ptr = inner.device_mut() as *mut bindings::v4l2_device;
+
+ device.v4l2_dev = v4l2_device_ptr;
+ drop(guard);
+
+ Ok(Self {
+ device,
+ _v4l2_device: v4l2_device,
+ _phantom: PhantomData,
+ })
+ }
+
+ fn _raw(&self) -> &bindings::video_device {
+ &self.device
+ }
+
+ fn raw_mut(&mut self) -> &mut bindings::video_device {
+ &mut self.device
+ }
+}
+
+impl<T> Drop for VideoDevice<T> {
+ fn drop(&mut self) {
+ unsafe { bindings::video_device_release(self.raw_mut()) }
+ }
+}
+
+/// Functions to be passed to the `fops` member.
+#[derive(Default)]
+#[allow(missing_docs)]
+pub struct V4l2FileOperations {
+ pub read: Option<
+ unsafe extern "C" fn(
+ arg1: *mut bindings::file,
+ arg2: *mut core::ffi::c_char,
+ arg3: usize,
+ arg4: *mut bindings::loff_t,
+ ) -> isize,
+ >,
+ pub poll: Option<
+ unsafe extern "C" fn(
+ arg1: *mut bindings::file,
+ arg2: *mut bindings::poll_table_struct,
+ ) -> bindings::__poll_t,
+ >,
+ pub mmap: Option<
+ unsafe extern "C" fn(
+ arg1: *mut bindings::file,
+ arg2: *mut bindings::vm_area_struct,
+ ) -> core::ffi::c_int,
+ >,
+ pub open: Option<unsafe extern "C" fn(arg1: *mut bindings::file) -> core::ffi::c_int>,
+}
+
+/// Functions to be passed to the `ioctl` member.
+#[derive(Default)]
+#[allow(missing_docs)]
+pub struct V4l2IoctlOperations {
+ pub reqbufs: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ b: *mut bindings::v4l2_requestbuffers,
+ ) -> core::ffi::c_int,
+ >,
+ pub querybuf: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ b: *mut bindings::v4l2_buffer,
+ ) -> core::ffi::c_int,
+ >,
+ pub qbuf: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ b: *mut bindings::v4l2_buffer,
+ ) -> core::ffi::c_int,
+ >,
+ pub expbuf: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ e: *mut bindings::v4l2_exportbuffer,
+ ) -> core::ffi::c_int,
+ >,
+ pub dqbuf: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ b: *mut bindings::v4l2_buffer,
+ ) -> core::ffi::c_int,
+ >,
+ pub create_bufs: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ b: *mut bindings::v4l2_create_buffers,
+ ) -> core::ffi::c_int,
+ >,
+ pub prepare_buf: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ b: *mut bindings::v4l2_buffer,
+ ) -> core::ffi::c_int,
+ >,
+ pub streamon: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ i: bindings::v4l2_buf_type,
+ ) -> core::ffi::c_int,
+ >,
+ pub streamoff: Option<
+ unsafe extern "C" fn(
+ file: *mut bindings::file,
+ fh: *mut core::ffi::c_void,
+ i: bindings::v4l2_buf_type,
+ ) -> core::ffi::c_int,
+ >,
+}
+
+/// The video registration struct. See `video_device_register!`.
+pub struct VideoRegistration<T: Driver> {
+ device: VideoDevice<T>,
+ registered: bool,
+ fops: bindings::v4l2_file_operations,
+ ioctls: bindings::v4l2_ioctl_ops,
+}
+
+impl<T: Driver> VideoRegistration<T> {
+ /// Creates a new `VideoRegistration`. The driver can manually choose
+ /// alternate ioctl implementations using the `ioctl_operations` argument.
+ pub fn new(
+ _parent: &dyn device::RawDevice,
+ v4l2_device: Arc<Mutex<V4l2Device>>,
+ file_operations: V4l2FileOperations,
+ ioctl_operations: V4l2IoctlOperations,
+ ) -> Result<Self> {
+ let video_device = VideoDevice::new(v4l2_device)?;
+
+ let fops = bindings::v4l2_file_operations {
+ owner: core::ptr::null_mut(),
+ read: file_operations.read,
+ write: None,
+ poll: file_operations.poll,
+ unlocked_ioctl: Some(bindings::video_ioctl2),
+ compat_ioctl32: None,
+ get_unmapped_area: None,
+ mmap: file_operations.mmap,
+ open: file_operations.open,
+ //file_operations::release() is not mandatory, let's try to release things on drop.
+ release: None,
+ };
+
+ let mut ioctls = *crate::media::v4l2::ioctls::OperationsVtable::<T>::build();
+
+ if let Some(op) = &ioctl_operations.reqbufs {
+ ioctls.vidioc_reqbufs = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.querybuf {
+ ioctls.vidioc_querybuf = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.qbuf {
+ ioctls.vidioc_qbuf = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.expbuf {
+ ioctls.vidioc_expbuf = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.dqbuf {
+ ioctls.vidioc_dqbuf = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.create_bufs {
+ ioctls.vidioc_create_bufs = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.prepare_buf {
+ ioctls.vidioc_prepare_buf = Some(*op)
+ }
+
+ if let Some(op) = &ioctl_operations.streamon {
+ ioctls.vidioc_streamon = Some(*op);
+ }
+
+ if let Some(op) = &ioctl_operations.streamoff {
+ ioctls.vidioc_streamoff = Some(*op);
+ }
+
+ Ok(Self {
+ device: video_device,
+ registered: false,
+ fops,
+ ioctls,
+ })
+ }
+
+ /// Registers a video device with the rest of the kernel.
+ ///
+ /// Users are encouraged to use the [`video_device_register`] macro because it automatically
+ /// picks up THIS_MODULE.
+ pub fn register(
+ self: Pin<&mut Self>,
+ data: T::DriverData,
+ vfl_devnode_type: VflDevNodeType,
+ nr: i32,
+ module: &'static ThisModule,
+ capabilities: &[capabilities::Capabilities],
+ ) -> Result {
+ if self.registered {
+ // Already registered.
+ return Err(EINVAL);
+ }
+
+ // SAFETY: We never move out of `this` in this method.
+ let this = unsafe { self.get_unchecked_mut() };
+ let video_device = this.device.raw_mut();
+
+ let mut caps = 0;
+ for capability in capabilities {
+ caps |= *capability as u32;
+ }
+
+ video_device.device_caps = caps;
+
+ // We can release our resources on drop.
+ video_device.release = Some(bindings::video_device_release_empty);
+
+ let data_pointer = <T::DriverData as ForeignOwnable>::into_foreign(data);
+
+ this.fops.owner = module.0;
+ video_device.fops = &this.fops;
+ video_device.ioctl_ops = &this.ioctls;
+
+ // SAFETY: video_device was properly allocated during video_register_device
+ // Use a single unsafe block based on the safety comment above to avoid
+ // repetition of unsafe on pointer accesses below.
+ unsafe {
+ let ret = bindings::__video_register_device(
+ video_device,
+ vfl_devnode_type as _,
+ nr,
+ 1,
+ module.0,
+ );
+
+ if ret < 0 {
+ // Reassemble into ForeignOwnable so it can be dropped.
+ // SAFETY: `data_pointer` was returned by `into_foreign` above.
+ T::DriverData::from_foreign(data_pointer);
+ return Err(Error::from_kernel_errno(ret));
+ }
+
+ let video_device_parent = &mut video_device.dev;
+ video_device_parent.driver_data = data_pointer as _;
+ }
+
+ this.registered = true;
+ Ok(())
+ }
+}
+
+impl<T: Driver> Drop for VideoRegistration<T> {
+ fn drop(&mut self) {
+ if self.registered {
+ // SAFETY: device has been properly initialized as per the type invariant.
+ let video_device_parent = &mut self.device.raw_mut().dev;
+
+ // Obtain a pointer to this before freeing, so it can be assembled
+ // back into ForeignOwnable and then dropped.
+ let data_pointer = video_device_parent.driver_data;
+
+ // SAFETY: pointer was properly allocated in VideoDevice::new()
+ unsafe { bindings::video_unregister_device(self.device.raw_mut()) }
+
+ // Free data as well.
+ // SAFETY: `data_pointer` was returned by `into_foreign` during registration.
+ unsafe { <T::DriverData as ForeignOwnable>::from_foreign(data_pointer) };
+ }
+ }
+}
+
+// SAFETY: `Registration` doesn't offer any methods or access to fields when
+// shared between threads or CPUs, so it is safe to share it.
+unsafe impl<T: Driver> Sync for VideoRegistration<T> {}
+
+// SAFETY: Registration and unregistration from the v4l2 subsystem can happen
+// from any thread. Additionally, `T::Data` (which is dropped during
+// unregistration) is `Send`, so it is ok to move `Registration` to different
+// threads.
+#[allow(clippy::non_send_fields_in_send_ty)]
+unsafe impl<T: Driver> Send for VideoRegistration<T> {}
+
+/// Registers a video device with the rest of the kernel.
+///
+/// It automatically picks up THIS_MODULE.
+#[allow(clippy::crate_in_macro_def)]
+#[macro_export]
+macro_rules! video_device_register {
+ ($reg:expr, $data:expr, $vfl_devnode_type:expr, $nr:expr, $capabilities:expr $(,)?) => {{
+ $crate::media::v4l2::dev::VideoRegistration::register(
+ $reg,
+ $data,
+ $vfl_devnode_type,
+ $nr,
+ &crate::THIS_MODULE,
+ $capabilities,
+ )
+ }};
+}
diff --git a/rust/kernel/media/v4l2/device.rs b/rust/kernel/media/v4l2/device.rs
new file mode 100644
index 000000000000..d7fafab787a6
--- /dev/null
+++ b/rust/kernel/media/v4l2/device.rs
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0
+#![allow(missing_docs)]
+
+//! V4L2 Device support helper.
+//!
+//! C header: [`include/media/v4l2-device.h`](../../../../include/media/v4l2-device.h)
+
+use core::cell::UnsafeCell;
+use core::pin::Pin;
+
+use crate::device;
+use crate::error;
+use crate::prelude::*;
+use crate::sync::smutex::Mutex;
+use crate::sync::Arc;
+use crate::ThisModule;
+
+/// A V4L2 device.
+pub struct V4l2Device {
+ /// The underlying `struct v4l2_device` in C.
+ device: UnsafeCell<bindings::v4l2_device>,
+}
+
+impl V4l2Device {
+ fn new() -> Self {
+ Self {
+ device: Default::default(),
+ }
+ }
+
+ pub(crate) fn device_mut(&mut self) -> &mut bindings::v4l2_device {
+ self.device.get_mut()
+ }
+}
+
+pub struct V4l2Registration {
+ device: Arc<Mutex<V4l2Device>>,
+ registered: bool,
+}
+
+impl V4l2Registration {
+ pub fn new() -> Result<Self> {
+ let device = Arc::try_new(Mutex::new(V4l2Device::new()))?;
+ Ok(Self {
+ device,
+ registered: false,
+ })
+ }
+
+ /// Registers a video device with the rest of the kernel.
+ ///
+ /// Users are encouraged to use the [`v4l2_device_register`] macro because it automatically
+ /// picks up THIS_MODULE.
+ pub fn register(
+ self: Pin<&mut Self>,
+ _module: &'static ThisModule,
+ parent: &dyn device::RawDevice,
+ ) -> Result {
+ if self.registered {
+ // Already registered.
+ return Err(EINVAL);
+ }
+
+ // SAFETY: We never move out of `this` in this method.
+ let this = unsafe { self.get_unchecked_mut() };
+
+ let device = UnsafeCell::new(bindings::v4l2_device::default());
+
+ // SAFETY: We know that `RawDevice` is valid due to its safety
+ // requirements. Furthermore, `device` has been stack-allocated above
+ // and thus it is valid.
+ let res = unsafe { bindings::v4l2_device_register(parent.raw_device(), device.get()) };
+ error::to_result(res)?;
+
+ this.device.as_ref().lock().device = device;
+ this.registered = true;
+ Ok(())
+ }
+
+ pub fn device(&self) -> Arc<Mutex<V4l2Device>> {
+ self.device.clone()
+ }
+}
+
+impl Drop for V4l2Registration {
+ fn drop(&mut self) {
+ if self.registered {
+ let mut device = self.device.as_ref().lock();
+ let v4l2_dev = &mut *device;
+
+ unsafe { bindings::v4l2_device_unregister(v4l2_dev.device.get()) }
+ }
+ }
+}
+
+// SAFETY: The only field made available by `Registration` is Send + Sync.
+// Other than that, it doesn't offer any methods or access to fields when shared
+// between threads or CPUs, so it is safe to share it.
+unsafe impl Sync for V4l2Registration {}
+
+// SAFETY: Registration and unregistration from the v4l2 subsystem can happen
+// from any thread.
+#[allow(clippy::non_send_fields_in_send_ty)]
+unsafe impl Send for V4l2Registration {}
+
+/// Registers a V4L2 device with the rest of the kernel.
+///
+/// It automatically picks up THIS_MODULE.
+#[allow(clippy::crate_in_macro_def)]
+#[macro_export]
+macro_rules! v4l2_device_register {
+ ($reg:expr, $parent:expr $(,)?) => {{
+ $crate::media::v4l2::device::V4l2Registration::register($reg, &crate::THIS_MODULE, $parent)
+ }};
+}
diff --git a/rust/kernel/media/v4l2/ioctls.rs b/rust/kernel/media/v4l2/ioctls.rs
new file mode 100644
index 000000000000..7c2b45c45ad6
--- /dev/null
+++ b/rust/kernel/media/v4l2/ioctls.rs
@@ -0,0 +1,608 @@
+// SPDX-License-Identifier: GPL-2.0
+#![allow(missing_docs)]
+
+//! V4L2 Ioctls
+//!
+//! C header: [`include/media/v4l2-ioctls.h`](../../../../include/media/v4l2-ioctls.h)
+
+use bindings::*;
+use core::marker::PhantomData;
+
+use crate::error::from_kernel_result;
+use crate::media::v4l2;
+use crate::prelude::*;
+use crate::ForeignOwnable;
+
+#[vtable]
+pub trait Operations {
+ type PrivateData: ForeignOwnable;
+
+ fn vidioc_querycap(
+ _private_data: &mut Self::PrivateData,
+ _cap: v4l2::capabilities::CapabilitiesRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_enum_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatDescRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_enum_framesizes(
+ _private_data: &mut Self::PrivateData,
+ _fsize: v4l2::framesize::FrameSizeRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_g_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_s_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_try_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_reqbufs(
+ _private_data: &mut Self::PrivateData,
+ _b: v4l2::mmap::RequestBuffersRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_querybuf(_private_data: &mut Self::PrivateData, _b: v4l2::mmap::BufferRef) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_qbuf(_private_data: &mut Self::PrivateData, _b: v4l2::mmap::BufferRef) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_expbuf(
+ _private_data: &mut Self::PrivateData,
+ _e: v4l2::mmap::ExportBufferRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_dqbuf(_private_data: &mut Self::PrivateData, _b: v4l2::mmap::BufferRef) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_create_bufs(
+ _private_data: &mut Self::PrivateData,
+ _b: v4l2::mmap::CreateBuffersRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_prepare_buf(
+ _private_data: &mut Self::PrivateData,
+ _b: v4l2::mmap::BufferRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_streamon(_private_data: &mut Self::PrivateData, _i: v4l2::enums::BufType) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_streamoff(_private_data: &mut Self::PrivateData, _i: v4l2::enums::BufType) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_enum_input(
+ _private_data: &mut Self::PrivateData,
+ _inp: v4l2::inputs::InputRef,
+ ) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_g_input(_private_data: &mut Self::PrivateData, _i: &mut u32) -> Result {
+ Ok(())
+ }
+
+ fn vidioc_s_input(_private_data: &mut Self::PrivateData, _i: u32) -> Result {
+ Ok(())
+ }
+}
+
+pub(crate) struct OperationsVtable<T: Operations>(PhantomData<T>);
+
+impl<T: Operations> OperationsVtable<T> {
+ fn private_data(file: *mut file) -> crate::ScopeGuard<T::PrivateData, fn(T::PrivateData)> {
+ // SAFETY: This was set during the video device registration process.
+ let video_device = unsafe { bindings::video_devdata(file) };
+ let data_ptr = unsafe { (*video_device).dev.driver_data };
+
+ // SAFETY: This was set during the video device registration process and
+ // now is being passed in by the kernel. We ensure exclusive access
+ // while the guard is alive.
+ unsafe { T::PrivateData::borrow_mut(data_ptr) }
+ }
+
+ unsafe extern "C" fn vidioc_querycap_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ cap: *mut v4l2_capability,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let cap = unsafe { v4l2::capabilities::CapabilitiesRef::from_ptr(cap) } ;
+ T::vidioc_querycap(&mut private_data, cap)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_enum_fmt_vid_cap_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ f: *mut v4l2_fmtdesc,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let fmt_desc = unsafe { v4l2::format::FormatDescRef::from_ptr(f) } ;
+ T::vidioc_enum_fmt_vid_cap(&mut private_data, fmt_desc)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_enum_framesizes(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ fsize: *mut v4l2_frmsizeenum,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let framesizes = unsafe { v4l2::framesize::FrameSizeRef::from_ptr(fsize) } ;
+ T::vidioc_enum_framesizes(&mut private_data, framesizes)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_g_fmt_vid_cap_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ f: *mut v4l2_format,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let fmt = unsafe { v4l2::format::FormatRef::from_ptr(f) } ;
+ T::vidioc_g_fmt_vid_cap(&mut private_data, fmt)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_s_fmt_vid_cap_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ f: *mut v4l2_format,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let fmt = unsafe { v4l2::format::FormatRef::from_ptr(f) } ;
+ T::vidioc_s_fmt_vid_cap(&mut private_data, fmt)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_try_fmt_vid_cap_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ f: *mut v4l2_format,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let fmt = unsafe { v4l2::format::FormatRef::from_ptr(f) } ;
+ T::vidioc_try_fmt_vid_cap(&mut private_data, fmt)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_reqbufs_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ b: *mut v4l2_requestbuffers,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let rb = unsafe { v4l2::mmap::RequestBuffersRef::from_ptr(b) } ;
+ T::vidioc_reqbufs(&mut private_data, rb)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_querybuf_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ b: *mut v4l2_buffer,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let buf = unsafe { v4l2::mmap::BufferRef::from_ptr(b) } ;
+ T::vidioc_querybuf(&mut private_data, buf)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_qbuf_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ b: *mut v4l2_buffer,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let buf = unsafe { v4l2::mmap::BufferRef::from_ptr(b) } ;
+ T::vidioc_qbuf(&mut private_data, buf)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_expbuf_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ e: *mut v4l2_exportbuffer,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let expbuf = unsafe { v4l2::mmap::ExportBufferRef::from_ptr(e) } ;
+ T::vidioc_expbuf(&mut private_data, expbuf)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_dqbuf_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ b: *mut v4l2_buffer,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let buf = unsafe { v4l2::mmap::BufferRef::from_ptr(b) } ;
+ T::vidioc_dqbuf(&mut private_data, buf)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_create_bufs_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ b: *mut v4l2_create_buffers,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let createbuf = unsafe { v4l2::mmap::CreateBuffersRef::from_ptr(b) } ;
+ T::vidioc_create_bufs(&mut private_data, createbuf)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_prepare_buf_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ b: *mut v4l2_buffer,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let buf = unsafe { v4l2::mmap::BufferRef::from_ptr(b) } ;
+ T::vidioc_prepare_buf(&mut private_data, buf)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_streamon_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ i: v4l2_buf_type,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let buf_type = v4l2::enums::BufType::try_from(i)?;
+ T::vidioc_streamon(&mut private_data, buf_type)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_streamoff_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ i: v4l2_buf_type,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let buf_type = v4l2::enums::BufType::try_from(i)?;
+ T::vidioc_streamoff(&mut private_data, buf_type)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_enum_input_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ inp: *mut v4l2_input,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ let input = unsafe { v4l2::inputs::InputRef::from_ptr(inp) };
+ T::vidioc_enum_input(&mut private_data, input)?;
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_g_input_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ i: *mut core::ffi::c_uint,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+ let mut val = 0;
+
+ T::vidioc_g_input(&mut private_data, &mut val)?;
+
+ // SAFETY: This pointer is passed in and checked by the kernel.
+ unsafe {*i = val; }
+ Ok(0)
+ }
+ }
+
+ unsafe extern "C" fn vidioc_s_input_callback(
+ file: *mut file,
+ _fh: *mut core::ffi::c_void,
+ i: core::ffi::c_uint,
+ ) -> core::ffi::c_int {
+ from_kernel_result! {
+ let mut private_data = Self::private_data(file);
+
+ T::vidioc_s_input(&mut private_data, i)?;
+
+ Ok(0)
+ }
+ }
+
+ const VTABLE: bindings::v4l2_ioctl_ops = bindings::v4l2_ioctl_ops {
+ vidioc_querycap: if T::HAS_VIDIOC_QUERYCAP {
+ Some(Self::vidioc_querycap_callback)
+ } else {
+ None
+ },
+ vidioc_enum_fmt_vid_cap: if T::HAS_VIDIOC_ENUM_FMT_VID_CAP {
+ Some(Self::vidioc_enum_fmt_vid_cap_callback)
+ } else {
+ None
+ },
+ vidioc_enum_fmt_vid_overlay: None,
+ vidioc_enum_fmt_vid_out: None,
+ vidioc_enum_fmt_sdr_cap: None,
+ vidioc_enum_fmt_sdr_out: None,
+ vidioc_enum_fmt_meta_cap: None,
+ vidioc_enum_fmt_meta_out: None,
+ vidioc_g_fmt_vid_cap: if T::HAS_VIDIOC_G_FMT_VID_CAP {
+ Some(Self::vidioc_g_fmt_vid_cap_callback)
+ } else {
+ None
+ },
+ vidioc_g_fmt_vid_overlay: None,
+ vidioc_g_fmt_vid_out: None,
+ vidioc_g_fmt_vid_out_overlay: None,
+ vidioc_g_fmt_vbi_cap: None,
+ vidioc_g_fmt_vbi_out: None,
+ vidioc_g_fmt_sliced_vbi_cap: None,
+ vidioc_g_fmt_sliced_vbi_out: None,
+ vidioc_g_fmt_vid_cap_mplane: None,
+ vidioc_g_fmt_vid_out_mplane: None,
+ vidioc_g_fmt_sdr_cap: None,
+ vidioc_g_fmt_sdr_out: None,
+ vidioc_g_fmt_meta_cap: None,
+ vidioc_g_fmt_meta_out: None,
+ vidioc_s_fmt_vid_cap: if T::HAS_VIDIOC_S_FMT_VID_CAP {
+ Some(Self::vidioc_s_fmt_vid_cap_callback)
+ } else {
+ None
+ },
+ vidioc_s_fmt_vid_overlay: None,
+ vidioc_s_fmt_vid_out: None,
+ vidioc_s_fmt_vid_out_overlay: None,
+ vidioc_s_fmt_vbi_cap: None,
+ vidioc_s_fmt_vbi_out: None,
+ vidioc_s_fmt_sliced_vbi_cap: None,
+ vidioc_s_fmt_sliced_vbi_out: None,
+ vidioc_s_fmt_vid_cap_mplane: None,
+ vidioc_s_fmt_vid_out_mplane: None,
+ vidioc_s_fmt_sdr_cap: None,
+ vidioc_s_fmt_sdr_out: None,
+ vidioc_s_fmt_meta_cap: None,
+ vidioc_s_fmt_meta_out: None,
+ vidioc_try_fmt_vid_cap: if T::HAS_VIDIOC_TRY_FMT_VID_CAP {
+ Some(Self::vidioc_try_fmt_vid_cap_callback)
+ } else {
+ None
+ },
+ vidioc_try_fmt_vid_overlay: None,
+ vidioc_try_fmt_vid_out: None,
+ vidioc_try_fmt_vid_out_overlay: None,
+ vidioc_try_fmt_vbi_cap: None,
+ vidioc_try_fmt_vbi_out: None,
+ vidioc_try_fmt_sliced_vbi_cap: None,
+ vidioc_try_fmt_sliced_vbi_out: None,
+ vidioc_try_fmt_vid_cap_mplane: None,
+ vidioc_try_fmt_vid_out_mplane: None,
+ vidioc_try_fmt_sdr_cap: None,
+ vidioc_try_fmt_sdr_out: None,
+ vidioc_try_fmt_meta_cap: None,
+ vidioc_try_fmt_meta_out: None,
+ vidioc_reqbufs: if T::HAS_VIDIOC_REQBUFS {
+ Some(Self::vidioc_reqbufs_callback)
+ } else {
+ None
+ },
+ vidioc_querybuf: if T::HAS_VIDIOC_QUERYBUF {
+ Some(Self::vidioc_querybuf_callback)
+ } else {
+ None
+ },
+ vidioc_qbuf: if T::HAS_VIDIOC_QBUF {
+ Some(Self::vidioc_qbuf_callback)
+ } else {
+ None
+ },
+ vidioc_expbuf: if T::HAS_VIDIOC_EXPBUF {
+ Some(Self::vidioc_expbuf_callback)
+ } else {
+ None
+ },
+ vidioc_dqbuf: if T::HAS_VIDIOC_DQBUF {
+ Some(Self::vidioc_dqbuf_callback)
+ } else {
+ None
+ },
+ vidioc_create_bufs: if T::HAS_VIDIOC_CREATE_BUFS {
+ Some(Self::vidioc_create_bufs_callback)
+ } else {
+ None
+ },
+ vidioc_prepare_buf: if T::HAS_VIDIOC_PREPARE_BUF {
+ Some(Self::vidioc_prepare_buf_callback)
+ } else {
+ None
+ },
+ vidioc_overlay: None,
+ vidioc_g_fbuf: None,
+ vidioc_s_fbuf: None,
+ vidioc_streamon: if T::HAS_VIDIOC_STREAMON {
+ Some(Self::vidioc_streamon_callback)
+ } else {
+ None
+ },
+ vidioc_streamoff: if T::HAS_VIDIOC_STREAMOFF {
+ Some(Self::vidioc_streamoff_callback)
+ } else {
+ None
+ },
+ vidioc_g_std: None,
+ vidioc_s_std: None,
+ vidioc_querystd: None,
+ vidioc_enum_input: if T::HAS_VIDIOC_ENUM_INPUT {
+ Some(Self::vidioc_enum_input_callback)
+ } else {
+ None
+ },
+ vidioc_g_input: if T::HAS_VIDIOC_G_INPUT {
+ Some(Self::vidioc_g_input_callback)
+ } else {
+ None
+ },
+ vidioc_s_input: if T::HAS_VIDIOC_S_INPUT {
+ Some(Self::vidioc_s_input_callback)
+ } else {
+ None
+ },
+ vidioc_enum_output: None,
+ vidioc_g_output: None,
+ vidioc_s_output: None,
+ vidioc_queryctrl: None,
+ vidioc_query_ext_ctrl: None,
+ vidioc_g_ctrl: None,
+ vidioc_s_ctrl: None,
+ vidioc_g_ext_ctrls: None,
+ vidioc_s_ext_ctrls: None,
+ vidioc_try_ext_ctrls: None,
+ vidioc_querymenu: None,
+ vidioc_enumaudio: None,
+ vidioc_g_audio: None,
+ vidioc_s_audio: None,
+ vidioc_enumaudout: None,
+ vidioc_g_audout: None,
+ vidioc_s_audout: None,
+ vidioc_g_modulator: None,
+ vidioc_s_modulator: None,
+ vidioc_g_pixelaspect: None,
+ vidioc_g_selection: None,
+ vidioc_s_selection: None,
+ vidioc_g_jpegcomp: None,
+ vidioc_s_jpegcomp: None,
+ vidioc_g_enc_index: None,
+ vidioc_encoder_cmd: None,
+ vidioc_try_encoder_cmd: None,
+ vidioc_decoder_cmd: None,
+ vidioc_try_decoder_cmd: None,
+ vidioc_g_parm: None,
+ vidioc_s_parm: None,
+ vidioc_g_tuner: None,
+ vidioc_s_tuner: None,
+ vidioc_g_frequency: None,
+ vidioc_s_frequency: None,
+ vidioc_enum_freq_bands: None,
+ vidioc_g_sliced_vbi_cap: None,
+ vidioc_log_status: None,
+ vidioc_s_hw_freq_seek: None,
+ vidioc_enum_framesizes: if T::HAS_VIDIOC_ENUM_FRAMESIZES {
+ Some(Self::vidioc_enum_framesizes)
+ } else {
+ None
+ },
+ vidioc_enum_frameintervals: None,
+ vidioc_s_dv_timings: None,
+ vidioc_g_dv_timings: None,
+ vidioc_query_dv_timings: None,
+ vidioc_enum_dv_timings: None,
+ vidioc_dv_timings_cap: None,
+ vidioc_g_edid: None,
+ vidioc_s_edid: None,
+ vidioc_subscribe_event: None,
+ vidioc_unsubscribe_event: None,
+ vidioc_default: None,
+ };
+
+ pub(crate) fn build() -> &'static bindings::v4l2_ioctl_ops {
+ &Self::VTABLE
+ }
+}
diff --git a/rust/kernel/media/v4l2/mod.rs b/rust/kernel/media/v4l2/mod.rs
index 77864640f19e..c60083266202 100644
--- a/rust/kernel/media/v4l2/mod.rs
+++ b/rust/kernel/media/v4l2/mod.rs
@@ -3,8 +3,11 @@
//! Abstractions for include/media/v4l2-*.h

pub mod capabilities;
+pub mod dev;
+pub mod device;
pub mod enums;
pub mod format;
pub mod framesize;
pub mod inputs;
+pub mod ioctls;
pub mod mmap;
--
2.40.0

2023-04-06 22:00:58

[permalink] [raw]

Subject: [PATCH 6/6] rust: media: add v4l2 rust sample

Add a v4l2 rust sample. The sample driver showcases the current support
available in the media module. It also proves that the device will
indeed probe by printing a message to the terminal indicating that
no error took place.

Signed-off-by: Daniel Almeida <[email protected]>
---
samples/rust/Kconfig | 11 ++
samples/rust/Makefile | 1 +
samples/rust/rust_v4l2.rs | 403 ++++++++++++++++++++++++++++++++++++++
3 files changed, 415 insertions(+)
create mode 100644 samples/rust/rust_v4l2.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 189c10ced6d4..4bdea8210ae0 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -163,4 +163,15 @@ config SAMPLE_RUST_SELFTESTS

If unsure, say N.

+config SAMPLE_RUST_V4L2
+ tristate "V4L2 driver"
+ depends on V4L_PLATFORM_DRIVERS
+ depends on VIDEO_DEV
+ select VIDEOBUF2_DMA_SG
+
+ help
+ This option builds the V4L2 example.
+
+ If unsure, say N.
+
endif # SAMPLES_RUST
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 420bcefeb082..5ffa621f3968 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -15,5 +15,6 @@ obj-$(CONFIG_SAMPLE_RUST_NETFILTER) += rust_netfilter.o
obj-$(CONFIG_SAMPLE_RUST_ECHO_SERVER) += rust_echo_server.o
obj-$(CONFIG_SAMPLE_RUST_FS) += rust_fs.o
obj-$(CONFIG_SAMPLE_RUST_SELFTESTS) += rust_selftests.o
+obj-$(CONFIG_SAMPLE_RUST_V4L2) += rust_v4l2.o

subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS) += hostprogs
diff --git a/samples/rust/rust_v4l2.rs b/samples/rust/rust_v4l2.rs
new file mode 100644
index 000000000000..6742af36ac0a
--- /dev/null
+++ b/samples/rust/rust_v4l2.rs
@@ -0,0 +1,403 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust V4L2 sample.
+
+use core::cell::UnsafeCell;
+use core::clone::Clone;
+
+use kernel::bindings;
+use kernel::error;
+use kernel::media::v4l2;
+use kernel::media::videobuf2;
+use kernel::platform;
+use kernel::prelude::*;
+use kernel::sync::smutex::Mutex;
+use kernel::sync::Arc;
+
+pub(crate) struct Driver {
+ _reg: Pin<Box<kernel::driver::Registration<kernel::platform::Adapter<Self>>>>,
+ _pdev: Pin<Box<PlatformDevice>>,
+}
+
+impl v4l2::dev::Driver for Driver {
+ type DriverData = Arc<DeviceData>;
+}
+
+pub(crate) struct Registrations {
+ video: Pin<Box<v4l2::dev::VideoRegistration<Driver>>>,
+ v4l2: Pin<Box<v4l2::device::V4l2Registration>>,
+}
+
+type DeviceData = kernel::device::Data<Registrations, (), V4l2Data>;
+
+pub(crate) struct V4l2Data {
+ pub(crate) _device: Arc<kernel::device::Device>,
+ _vb2_queue: Mutex<videobuf2::core::Queue<Driver>>,
+}
+
+#[vtable]
+impl videobuf2::core::QueueOperations for Driver {
+ type DriverSpecificData = Vb2QueueData;
+
+ type PrivateData = Box<videobuf2::core::PrivateData<bindings::vb2_v4l2_buffer, Vb2QueueData>>;
+
+ fn queue_setup(
+ _queue: &videobuf2::core::QueueRef,
+ _private_data: &mut Self::PrivateData,
+ _num_buffers: &mut u32,
+ _num_planes: &mut u32,
+ _sizes: &mut [u32],
+ ) -> Result {
+ let _vb2_queue_data = _private_data.driver_specific_mut();
+ pr_debug!("queue_setup called.\n");
+ Ok(())
+ }
+
+ fn start_streaming(
+ _queue: &videobuf2::core::QueueRef,
+ _private_data: &mut Self::PrivateData,
+ _count: u32,
+ ) -> Result {
+ pr_info!("start_streaming called.\n");
+ Ok(())
+ }
+
+ fn stop_streaming(_queue: &videobuf2::core::QueueRef, _private_data: &mut Self::PrivateData) {
+ pr_info!("stop_streaming called.\n");
+ }
+
+ fn buf_init(
+ _buffer: &videobuf2::core::Buffer,
+ _private_data: &mut Self::PrivateData,
+ ) -> Result {
+ pr_info!("buf_init called.\n");
+ Ok(())
+ }
+
+ fn buf_prepare(
+ _buffer: &videobuf2::core::Buffer,
+ _private_data: &mut Self::PrivateData,
+ ) -> Result {
+ pr_info!("buf_prepare called.\n");
+ Ok(())
+ }
+
+ fn buf_cleanup(_buffer: &videobuf2::core::Buffer, _private_data: &mut Self::PrivateData) {
+ pr_info!("buf_cleanup called.\n");
+ }
+
+ fn buf_queue(_buffer: &videobuf2::core::Buffer, _private_data: &mut Self::PrivateData) {
+ pr_info!("buf_queue called.\n");
+ }
+}
+
+pub(crate) struct Vb2QueueData {}
+
+impl platform::Driver for Driver {
+ type Data = Arc<DeviceData>;
+
+ kernel::define_of_id_table! {(), [
+ (kernel::of::DeviceId::Compatible(b"v4l2"), None),
+ ]}
+
+ fn probe(
+ pdev: &mut platform::Device,
+ _: core::option::Option<&Self::IdInfo>,
+ ) -> Result<Self::Data> {
+ let dev = kernel::device::Device::from_dev(pdev);
+
+ pr_debug!("V4L2 Rust Sample Probing!\n");
+
+ let v4l2_reg = v4l2::device::V4l2Registration::new()?;
+
+ let fops = v4l2::dev::V4l2FileOperations {
+ open: Some(bindings::v4l2_fh_open),
+ poll: Some(bindings::vb2_fop_poll),
+ mmap: Some(bindings::vb2_fop_mmap),
+ read: Some(bindings::vb2_fop_read),
+ };
+
+ let ioctls = v4l2::dev::V4l2IoctlOperations {
+ reqbufs: Some(bindings::vb2_ioctl_reqbufs),
+ querybuf: Some(bindings::vb2_ioctl_querybuf),
+ qbuf: Some(bindings::vb2_ioctl_qbuf),
+ expbuf: Some(bindings::vb2_ioctl_expbuf),
+ dqbuf: Some(bindings::vb2_ioctl_dqbuf),
+ create_bufs: Some(bindings::vb2_ioctl_create_bufs),
+ prepare_buf: Some(bindings::vb2_ioctl_prepare_buf),
+ streamon: Some(bindings::vb2_ioctl_streamon),
+ streamoff: Some(bindings::vb2_ioctl_streamoff),
+ // prepare_streaming: None,
+ // unprepare_streaming: None,
+ };
+
+ let video_reg =
+ v4l2::dev::VideoRegistration::<Driver>::new(&dev, v4l2_reg.device(), fops, ioctls)?;
+
+ let dev = Arc::try_new(dev)?;
+
+ let io_modes = [
+ videobuf2::core::IoModes::Mmap,
+ videobuf2::core::IoModes::DmaBuf,
+ ];
+
+ let timestamp_flags = [v4l2::mmap::BufferFlag::TimestampMonotonic];
+
+ let queue_data = Vb2QueueData {};
+
+ let queue_data = Box::try_new(videobuf2::core::PrivateData::new(queue_data))?;
+ let queue_data = Some(queue_data);
+
+ let vb2_mutex = unsafe { kernel::sync::ffi_mutex::FfiMutex::new() };
+ let mut vb2_mutex = Pin::from(Box::try_new(vb2_mutex)?);
+ kernel::ffi_mutex_init!(vb2_mutex.as_mut(), "Vb2Queue::lock");
+
+ let create_info = videobuf2::core::QueueCreateInfo {
+ buf_type: v4l2::enums::BufType::VideoCapture,
+ io_modes: &io_modes,
+ dev: dev.clone(),
+ timestamp_flags: &timestamp_flags,
+ lock: Some(vb2_mutex),
+ min_buffers_needed: 1,
+ gfp_flags: bindings::___GFP_DMA32,
+ mem_ops: videobuf2::core::MemOps::DmaSg,
+ requires_requests: false,
+ supports_requests: false,
+ private_data: queue_data,
+ };
+
+ let vb2_queue = videobuf2::core::Queue::<Self>::new(create_info)?;
+
+ let data = kernel::new_device_data!(
+ Registrations {
+ video: Pin::new(Box::try_new(video_reg).unwrap()),
+ v4l2: Pin::new(Box::try_new(v4l2_reg).unwrap()),
+ },
+ (),
+ V4l2Data {
+ _device: dev,
+ _vb2_queue: kernel::sync::smutex::Mutex::new(vb2_queue),
+ },
+ "V4l2::Registrations"
+ )?;
+
+ let data = Arc::<DeviceData>::from(data);
+ let capabilities = [
+ v4l2::capabilities::Capabilities::VideoCapture,
+ v4l2::capabilities::Capabilities::Streaming,
+ ];
+
+ let mut pinned_regs = data.registrations().ok_or(ENXIO)?;
+ let mut pinned_regs_mut = pinned_regs.as_pinned_mut();
+
+ let dev = kernel::device::Device::from_dev(pdev);
+ kernel::v4l2_device_register!(pinned_regs_mut.v4l2.as_mut(), &dev)?;
+ pr_debug!("Sucessfully registered v4l2 device");
+
+ kernel::video_device_register!(
+ pinned_regs_mut.video.as_mut(),
+ data.clone(),
+ v4l2::dev::VflDevNodeType::Video,
+ -1,
+ &capabilities,
+ )?;
+
+ pr_info!("V4L2 Rust Sample Probed!\n");
+ drop(pinned_regs);
+ Ok(data)
+ }
+}
+
+#[vtable]
+impl v4l2::ioctls::Operations for Driver {
+ type PrivateData = Arc<DeviceData>;
+
+ fn vidioc_querycap(
+ _private_data: &mut Self::PrivateData,
+ _cap: v4l2::capabilities::CapabilitiesRef,
+ ) -> Result {
+ pr_info!("vidioc_querycap called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_enum_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ mut f: v4l2::format::FormatDescRef,
+ ) -> Result {
+ pr_info!("vidioc_enum_fmt_vid_cap called");
+ // Set NV12 to avoid spam by some apps when this module is loaded.
+ f.set_pixel_format(842094158);
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_enum_framesizes(
+ _private_data: &mut Self::PrivateData,
+ _fsize: v4l2::framesize::FrameSizeRef,
+ ) -> Result {
+ pr_info!("vidioc_enum_framesizes called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_g_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatRef,
+ ) -> Result {
+ pr_info!("vidioc_g_fmt_vid_cap called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_s_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatRef,
+ ) -> Result {
+ pr_info!("vidioc_s_fmt_vid_cap called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_try_fmt_vid_cap(
+ _private_data: &mut Self::PrivateData,
+ _f: v4l2::format::FormatRef,
+ ) -> Result {
+ pr_info!("vidioc_try_fmt_vid_cap called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_reqbufs(
+ _private_data: &mut Self::PrivateData,
+ _b: v4l2::mmap::RequestBuffersRef,
+ ) -> Result {
+ pr_info!("vidioc_reqbufs called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_querybuf(_private_data: &mut Self::PrivateData, _b: v4l2::mmap::BufferRef) -> Result {
+ pr_info!("vidioc_querybuf called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_qbuf(_private_data: &mut Self::PrivateData, _b: v4l2::mmap::BufferRef) -> Result {
+ pr_info!("vidioc_qbuf called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_expbuf(
+ _private_data: &mut Self::PrivateData,
+ _e: v4l2::mmap::ExportBufferRef,
+ ) -> Result {
+ pr_info!("vidioc_expbuf called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_dqbuf(_private_data: &mut Self::PrivateData, _b: v4l2::mmap::BufferRef) -> Result {
+ pr_info!("vidioc_dqbuf called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_create_bufs(
+ _private_data: &mut Self::PrivateData,
+ _b: v4l2::mmap::CreateBuffersRef,
+ ) -> Result {
+ pr_info!("vidioc_create_bufs called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_prepare_buf(
+ _private_data: &mut Self::PrivateData,
+ _b: v4l2::mmap::BufferRef,
+ ) -> Result {
+ pr_info!("vidioc_prepare_buf called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_streamon(_private_data: &mut Self::PrivateData, _i: v4l2::enums::BufType) -> Result {
+ pr_info!("vidioc_streamon called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_streamoff(_private_data: &mut Self::PrivateData, _i: v4l2::enums::BufType) -> Result {
+ pr_info!("vidioc_streamoff called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_enum_input(
+ _private_data: &mut Self::PrivateData,
+ _inp: v4l2::inputs::InputRef,
+ ) -> Result {
+ pr_info!("vidioc_enum_input called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_g_input(_private_data: &mut Self::PrivateData, _i: &mut u32) -> Result {
+ pr_info!("vidioc_g_input called");
+ core::result::Result::Ok(())
+ }
+
+ fn vidioc_s_input(_private_data: &mut Self::PrivateData, _i: u32) -> Result {
+ pr_info!("vidioc_s_input called");
+ core::result::Result::Ok(())
+ }
+}
+
+/// An owned platform device registered by a driver. This is useful for virtual
+/// drivers, such as this one, since they will not be probed by matching against
+/// the device tree.
+struct PlatformDevice(core::cell::UnsafeCell<bindings::platform_device>);
+
+// SAFETY: I assume _all_ devices should be OK to be moved to any thread.
+unsafe impl Send for PlatformDevice {}
+// SAFETY: Platform device does not expose its inner FFI type.
+unsafe impl Sync for PlatformDevice {}
+
+impl Drop for PlatformDevice {
+ fn drop(&mut self) {
+ // SAFETY: no references to this type are alive here.
+ let platform_device = self.0.get();
+ // SAFETY: an FFI call to a previously registered device.
+ unsafe { bindings::platform_device_unregister(platform_device) }
+ }
+}
+
+unsafe impl kernel::device::RawDevice for PlatformDevice {
+ fn raw_device(&self) -> *mut bindings::device {
+ // SAFETY: no references to this type are alive here.
+ let platform_device = self.0.get();
+ // SAFETY: pointer is valid since it was previously registered when the
+ // module was registered.
+ unsafe { &mut (*platform_device).dev as _ }
+ }
+}
+
+impl kernel::Module for Driver {
+ fn init(name: &'static CStr, module: &'static ThisModule) -> Result<Self> {
+ // Register as platform driver
+ let reg: Pin<Box<kernel::driver::Registration<kernel::platform::Adapter<Self>>>> =
+ kernel::platform::Registration::new_pinned(name, module)?;
+
+ let mut platform_device = kernel::bindings::platform_device::default();
+ let name = kernel::c_str!("rust_v4l2");
+ platform_device.name = name.as_char_ptr();
+
+ let platform_device = Box::try_new(PlatformDevice(UnsafeCell::new(platform_device)))?;
+ let mut platform_device = Pin::from(platform_device);
+
+ let platform_device = unsafe {
+ let res = bindings::platform_device_register(platform_device.as_mut().0.get());
+
+ error::to_result(res)?;
+ platform_device
+ };
+
+ Ok(Self {
+ _reg: reg,
+ _pdev: platform_device, /* This obviously must be kept alive as well, otherwise massive faults will ensure */
+ })
+ }
+}
+
+module! {
+ type: Driver,
+ name: "rust_v4l2",
+ author: "Daniel Almeida",
+ description: "Minimal Rust V4L2 example driver",
+ license: "GPL",
+}
--
2.40.0

2023-04-08 19:27:33

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

By the way, one of the things I dislike about this series is that
there's a needless distinction between

struct Foo(bindgen::foo)

vs

struct FooRef(*mut bindgen::foo)

This gets in the way of having an owned Foo embedded into a larger
struct. It also gets in the way of instantiating an owned Foo on the
stack.

My first thought was to use enums:

enum Foo {
Owned(bindgen::foo),
NotOwned(*mut bindgen::foo),
}

But that would mean that users would invariably pay the price for the
owned variant always, as enums use as much space as its largest
variant.

My current understanding is that we can move all the implementations to
traits, with a suitable bound on AsRef<bindings::foo> and
AsMut<bindings::foo>.

Here is a code example for the wrapper of bindings::v4l2_format (see
format.rs), which was extended to account for both owned and non-owned
bindgen types:

```
use core::cell::UnsafeCell;

/// The shared implementation between Format and FormatRef.
pub trait FormatImpl: AsRef<bindings::v4l2_format> +
AsMut<bindings::v4l2_format> {
/// Returns the `type_` field.
fn type_(&self) -> u32 {
self.as_ref().type_
}

/// Get the field `field` for the `pix` union member.
fn pix_field(&self) -> Result<enums::Field> {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
enums::Field::try_from(pix.field)
}

/// Get the field `width` for the `pix` union member.
fn pix_width(&self) -> u32 {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
pix.width
}

/// Get the field `height` for the `pix` union member.
fn pix_height(&self) -> u32 {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
pix.height
}

/// Get the field `pixelformat` for the `pix` union member.
fn pix_pixelformat(&self) -> u32 {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
pix.pixelformat
}

/// Get the field `bytesperline` for the `pix` union member.
fn pix_bytesperline(&self) -> u32 {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
pix.bytesperline
}

/// Get the field `sizeimage` for the `pix` union member.
fn pix_sizeimage(&self) -> u32 {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
pix.sizeimage
}

/// Get the field `colorspace` for the `pix` union member.
fn pix_colorspace(&self) -> Result<enums::Colorspace> {
let fmt = self.as_ref();
let pix = &unsafe { fmt.fmt.pix };
enums::Colorspace::try_from(pix.colorspace)
}

/// Set the field `field` for the `pix` union member.
fn set_pix_field(&mut self, field: enums::Field) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.field = field as u32;
}

/// Set the field `width` for the `pix` union member.
fn set_pix_width(&mut self, width: u32) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.width = width;
}

/// Set the field `height` for the `pix` union member.
fn set_pix_height(&mut self, height: u32) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.height = height;
}

/// Set the field `pixelformat` for the `pix` union member.
fn set_pix_pixel_format(&mut self, pixel_format: u32) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.pixelformat = pixel_format;
}

/// Set the field `bytesperline` for the `pix` union member.
fn set_pix_bytesperline(&mut self, bytesperline: u32) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.bytesperline = bytesperline;
}

/// Set the field `sizeimage` for the `pix` union member.
fn set_pix_sizeimage(&mut self, sizeimage: u32) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.sizeimage = sizeimage;
}

/// Set the field `sizeimage` for the `pix` union member.
fn set_pix_colorspace(&mut self, colorspace: enums::Colorspace) {
let fmt = self.as_mut();
let pix = &mut unsafe { fmt.fmt.pix };
pix.colorspace = colorspace as u32;
}
}

/// A wrapper over a pointer to `struct v4l2_format`.
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
pub struct FormatRef(*mut bindings::v4l2_format);

impl FormatRef {
/// # Safety
/// The caller must ensure that `ptr` is valid and remains valid
for the lifetime of the
/// returned [`FormatRef`] instance.
pub unsafe fn from_ptr(ptr: *mut bindings::v4l2_format) -> Self {
Self(ptr)
}
}

impl AsRef<bindings::v4l2_format> for FormatRef {
fn as_ref(&self) -> &bindings::v4l2_format {
// SAFETY: ptr is safe during the lifetime of [`FormatRef`] as
per
// the safety requirement in `from_ptr()`
unsafe { self.0.as_ref().unwrap() }
}
}

impl AsMut<bindings::v4l2_format> for FormatRef {
fn as_mut(&mut self) -> &mut bindings::v4l2_format {
// SAFETY: ptr is safe during the lifetime of [`FormatRef`] as
per
// the safety requirement in `from_ptr()`
unsafe { self.0.as_mut().unwrap() }
}
}

impl FormatImpl for FormatRef {}

/// An owned version of `FormatRef`.
#[derive(Default)]
pub struct Format(UnsafeCell<bindings::v4l2_format>);

impl AsRef<bindings::v4l2_format> for Format {
fn as_ref(&self) -> &bindings::v4l2_format {
// SAFETY:
// It is safe to dereference the pointer since it is valid
whenever this type is instantiated.
// It is safe to cast this into a shared reference: because
this is the
// only method that returns &bindings::v4l2_format and because
we take
// &self, the compiler takes care of enforcing the Rust
reference rules for
// us. Thus, enforcing the safety guarantees of
UnsafeCell::get() by
// proxy.
unsafe { &*self.0.get() }
}
}

impl AsMut<bindings::v4l2_format> for Format {
fn as_mut(&mut self) -> &mut bindings::v4l2_format {
// SAFETY:
// It is safe to dereference the pointer since it is valid
whenever this type is instantiated.
// It is safe to cast this into an exclusive reference: because
this is the
// only method that returns &mut bindings::v4l2_format and
because we take
// &mut self, the compiler takes care of enforcing the Rust
reference rules for
// us. Thus, enforcing the safety guarantees of
UnsafeCell::get() by
// proxy.
unsafe { &mut *self.0.get() }
}
}

impl FormatImpl for Format {}

```

This makes it possible to:

- Share the implementations between Format and FormatRef,
- Have both Format (while paying the cost of storing the
bindings::v4l2_format member) and FormatRef (while paying the cost of
storing a pointer) separately.
- Be generic over Format and FormatRef when needed, e.g.:

```
fn some_fn(format: impl FormatImpl) {...}
```

Thoughts?

-- Daniel

2023-04-08 20:02:05

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 4/6/23 23:56, Daniel Almeida wrote:
> Hi all, this is my first attempt at adding Rust support to the
> media subsystem.
>
>
> Please let me know your thoughts.
>

Hi Daniel,

I think V4L2 should be written in primarily one language.

At first, I think Rust for V4L2 has no benefits for media drivers,
webcams, DVB-S/T/T2, pointing tablets and so on. You assume that all
code is running inside the kernel and needs to be perfect. But I think
you could just aswell implement the next USB webcam V4L2 driver in Perl
for that sake.

The reason for my point of view, is that I think most of the drivers in
media/ should run in user-space, and not inside the kernel. The driver
is killed when the device is detached, and all lost memory is reclaimed,
automagically. Then there exist proper methods to lock-down all
interfaces and file handles, so that device drivers will not do any
harm, even if exploited. For example the Capsicum library, I'm using
FreeBSD.

Debugging stuff using GDB in user-space, is so much more convenient than
debugging stuff inside the kernel. And the development time is much faster.

The example of secure V4L2 programming is already here:
https://github.com/hselasky/webcamd

I would rather like more drive on that, than flowing down the Rust
stream. Rust is cool, Java is cool, VM's are cool. The only bad about
cool things, is that they are so slow. For many years I completely
avoided C++ code for the sake it is very slow to compile, compared to
bare C code. And when looking at how Firefox is building using Rust, I
am a little worried, why we need so much code in there!

Engineering energy would be much more focused, if hardware vendors could
agree more about what binary formats to use for their device protocols,
than changing the coding language, so that now anyone can be let loose
to program in the Linux kernel without risking any damage.

The goal for Linux driver development should be fewer drivers and not
more. I'm glad if not everyone out there can do my job writing C-code
for device drivers. We don't need more people to mess around there
simply. I don't want Linux to become the next Microsoft, with gigabytes
of drivers which are never used for anything.

The webcamd daemon already is close to 6 MBytes big on amd64 on FreeBSD.
Inside there is support for 510 drivers (counting =y keywords), built
straight off Linus Torvalds:

cat config | grep CONFIG | grep "=y" | wc -l
510

ls -l `which webcamd`
-r-xr-xr-x 1 root wheel 5915016 Mar 30 19:09 /usr/local/sbin/webcamd

The USB video class is great, instead of tons of GSPCA devices, then
yeah, we don't need to drag around so much legacy binaries, just to make
everyone happy. What did Apple do? Custom PCI webcam devices? Why can't
they just stick with virtual USB devices, and then have a dual
configured device, one config for their own HD codec, and one config for
people like me, just needing the framebuffer.

You asked for a comment and now you got one!

--HPS

2023-04-09 14:13:05

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Hans! Thank you for chiming in!

There's a few things in your email that I disagree with and that I'd
like to
address.

> I think V4L2 should be written in primarily one language.

It is, in C. This series is about adding *bindings* to write *drivers*
in Rust
*for those interested*. The v4l2 core remains untouched, and I don't
think there
are any plans to introduce Rust outside of drivers in the kernel at
all, last I
heard.

> You assume that all code is running inside the kernel and needs to be
perfect.

No I do not assume that. In fact, Rust code is absolutely not
guaranteed to be
bug free and definitely not "perfect".

On the other hand, I would take Rust over C any day. Thus I am
contributing some
of the infrastructure to make this possible for me and for others.

IMHO I think you're approaching this from the wrong angle. It isn't
that Linux
*needs* Rust. It's more about providing a new and safer choice with
modern ergonomics for developers, is all.

> I would rather like more drive on that, than flowing down the Rust
stream.

These two things are not mutually exclusive :)

> Rust is cool, Java is cool, VM's are cool.

I don't see why Java and virtual machines are being brought into the
discussion
for this patchset here. And compilation times are a part of life,
sadly. Also,
can you substantiate your claim that Rust is slow?

> Engineering energy would be much more focused, if hardware vendors
could agree
more about what binary formats to use for their device protocols,

I understand, but my patchset is not to blame here. In fact, I have no
say at
all over these things.

> than changing the coding language

This simply is not what is happening here. Again this is about giving
kernel
developers another *option* of programming language, not about ditching
C.

> that now anyone can be let loose to program in the Linux kernel
without
risking any damage

Who's "anyone"? Plus the review process stays in place, so hardly any
changes to
code quality here.

> I'm glad if not everyone out there can do my job writing C-code for
device
drivers. We don't need more people to mess around there simply.

Ok we differ strongly here. In particular, I am totally neutral to your
first
statement.

The reality is that it isn't up to anyone to say who should or
shouldn't become
a kernel developer. The resources are out there for anyone to try, and
if
maintainers take in their patches, then that's the end of the story.

-- Daniel

2023-04-10 19:02:49

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Daniel,

On 4/9/23 16:10, Daniel Almeida wrote:
> Hi Hans! Thank you for chiming in!
>
> There's a few things in your email that I disagree with and that I'd
> like to
> address.
>
>> I think V4L2 should be written in primarily one language.
>
> It is, in C. This series is about adding *bindings* to write *drivers*
> in Rust
> *for those interested*. The v4l2 core remains untouched, and I don't
> think there
> are any plans to introduce Rust outside of drivers in the kernel at
> all, last I
> heard.

I see your point, but still I think it is better to have good examples,
than to say, there is a room for everything, just come here :-)

Adding a dependency to build the Rust compiler even to build one or two
V4L2 device drivers, would mean a lot to my small hselasky/webcamd
project. It already has to fetch a copy of the Linux kernel, and now has
to bootstrap Rust from stage0 to stageN. I personally say no. It's like
XCode unfortunately. I download 100's of GBytes of upgrades to XCode,
and barely upload one millionth worth of code back to Apple. It's not
good. Software developers shouldn't have to download more stuff than
they upload?

>
>> You assume that all code is running inside the kernel and needs to be
> perfect.
>
> No I do not assume that. In fact, Rust code is absolutely not
> guaranteed to be
> bug free and definitely not "perfect".

The definition of "bugs" may vary of course. I was thinking more like
stack exploits, missing validation of arrays and so on.

> On the other hand, I would take Rust over C any day. Thus I am
> contributing some
> of the infrastructure to make this possible for me and for others.
I must admit I'm not a Rust guy and don't see the advantages of Rust
like you do.

> IMHO I think you're approaching this from the wrong angle. It isn't
> that Linux
> *needs* Rust. It's more about providing a new and safer choice with
> modern ergonomics for developers, is all.

Why not move Linux-V4L2 drivers to user-space? In my opinion Rust is
much more easy to get going there than at the kernel level.

>
>> I would rather like more drive on that, than flowing down the Rust
> stream.
>
> These two things are not mutually exclusive :)
>
>> Rust is cool, Java is cool, VM's are cool.
>
> I don't see why Java and virtual machines are being brought into the
> discussion
> for this patchset here. And compilation times are a part of life,
> sadly. Also,
> can you substantiate your claim that Rust is slow?

Rust is slow based on my observations building Firefox from sources. The
Rust compiler spends a significant amount of time per source file.

>> Engineering energy would be much more focused, if hardware vendors
> could agree
> more about what binary formats to use for their device protocols,
>
> I understand, but my patchset is not to blame here. In fact, I have no
> say at
> all over these things.
>
>> than changing the coding language
>
> This simply is not what is happening here. Again this is about giving
> kernel
> developers another *option* of programming language, not about ditching
> C.

I think this option belongs in user-space and not Linux (the kernel).
More stuff should be moved there, that is my view.

>
>> that now anyone can be let loose to program in the Linux kernel
> without
> risking any damage
>
> Who's "anyone"? Plus the review process stays in place, so hardly any
> changes to
> code quality here.

Maybe the word "anyone" was a bit unclear in this regard. I take that
back for now.

>> I'm glad if not everyone out there can do my job writing C-code for
> device
> drivers. We don't need more people to mess around there simply.
>
> Ok we differ strongly here. In particular, I am totally neutral to your
> first
> statement.
>
> The reality is that it isn't up to anyone to say who should or
> shouldn't become
> a kernel developer. The resources are out there for anyone to try, and
> if
> maintainers take in their patches, then that's the end of the story.
The GPLv2 license should not be the only reason behind Linux developers
putting drivers in the kernel-space. I think moving more stuff to
user-space would benefit a greater purpose.

Summed up:

My main objection is Rust compiler support for _kernel_ V4L2 drivers. My
opinion it belongs to user-space for now and why not do something there
instead?

--HPS

2023-04-10 22:52:26

by Deborah Brouwer

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Thu, Apr 06, 2023 at 06:56:09PM -0300, Daniel Almeida wrote:
> Hi all, this is my first attempt at adding Rust support to the
> media subsystem.
>
> It adds just enough support to write a clone of the virtio-camera
> prototype written by my colleague, Dmitry Osipenko, available at [0].
>
> Basically, there's support for video_device_register,
> v4l2_device_register and for some ioctls in v4l2_ioctl_ops. There is
> also some initial vb2 support, alongside some wrappers for some types
> found in videodev2.h.
>
> I wrote a sample Rust driver just to prove that this probes, and
> that you get a message on dmesg whenever an ioctl is called.
>
> As there is no actual implementation for any of the ioctls, this module
> can misbehave with some programs. I can work around this in a future
> submission.
>
> Note that this is based on the rust branch, as opposed to rust-next. The
> reasoning is simple: I expect this series to just kickstart some
> discussion around the subject. Actual upstreaming can come up much
> later, at which point I can rebase on the rust branch.

Hi Daniel - I don't know if the 'rust branch' is common knowledge but
it helped me to know it was this:
https://github.com/Rust-for-Linux/linux

For what it's worth, I was able to get the V4L2 Rust sample probed
pretty easily following the quick start instructions
https://docs.kernel.org/rust/quick-start.html

Cool to see this working!
Deb

>
> Lastly, the origins of this series trace back to a v4l2 virtIO driver I
> was writing in Rust. As the project was eventually shelved for other
> reasons, I picked both the virtIO and the v4l2 bindings into their own
> patches which I am now in the process of submitting. This is to say that
> I tested this code with said driver and CrosVM in the past, and it
> worked ok.
>
> Please let me know your thoughts.
>
> [0]: https://gitlab.collabora.com/dmitry.osipenko/linux-kernel-rd/-/commit/055a2c322e931a8b388f864f1db81bbdfd525602
>
> Daniel Almeida (6):
> rust: media: add the media module
> rust: media: add initial videodev2.h abstractions
> rust: sync: introduce FfiMutex
> rust: media: videobuf2: add a videobuf2 abstraction
> rust: media: add {video|v4l2}_device_register support
> rust: media: add v4l2 rust sample
>
> rust/bindings/bindings_helper.h | 8 +
> rust/kernel/lib.rs | 2 +
> rust/kernel/media/mod.rs | 6 +
> rust/kernel/media/v4l2/capabilities.rs | 80 ++++
> rust/kernel/media/v4l2/dev.rs | 369 +++++++++++++++
> rust/kernel/media/v4l2/device.rs | 115 +++++
> rust/kernel/media/v4l2/enums.rs | 135 ++++++
> rust/kernel/media/v4l2/format.rs | 178 ++++++++
> rust/kernel/media/v4l2/framesize.rs | 176 +++++++
> rust/kernel/media/v4l2/inputs.rs | 104 +++++
> rust/kernel/media/v4l2/ioctls.rs | 608 +++++++++++++++++++++++++
> rust/kernel/media/v4l2/mmap.rs | 81 ++++
> rust/kernel/media/v4l2/mod.rs | 13 +
> rust/kernel/media/videobuf2/core.rs | 552 ++++++++++++++++++++++
> rust/kernel/media/videobuf2/mod.rs | 5 +
> rust/kernel/sync.rs | 1 +
> rust/kernel/sync/ffi_mutex.rs | 70 +++
> samples/rust/Kconfig | 11 +
> samples/rust/Makefile | 1 +
> samples/rust/rust_v4l2.rs | 403 ++++++++++++++++
> 20 files changed, 2918 insertions(+)
> create mode 100644 rust/kernel/media/mod.rs
> create mode 100644 rust/kernel/media/v4l2/capabilities.rs
> create mode 100644 rust/kernel/media/v4l2/dev.rs
> create mode 100644 rust/kernel/media/v4l2/device.rs
> create mode 100644 rust/kernel/media/v4l2/enums.rs
> create mode 100644 rust/kernel/media/v4l2/format.rs
> create mode 100644 rust/kernel/media/v4l2/framesize.rs
> create mode 100644 rust/kernel/media/v4l2/inputs.rs
> create mode 100644 rust/kernel/media/v4l2/ioctls.rs
> create mode 100644 rust/kernel/media/v4l2/mmap.rs
> create mode 100644 rust/kernel/media/v4l2/mod.rs
> create mode 100644 rust/kernel/media/videobuf2/core.rs
> create mode 100644 rust/kernel/media/videobuf2/mod.rs
> create mode 100644 rust/kernel/sync/ffi_mutex.rs
> create mode 100644 samples/rust/rust_v4l2.rs
>
> --
> 2.40.0
>

2023-04-11 00:01:03

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Mon, Apr 10, 2023 at 8:59 PM Hans Petter Selasky <[email protected]> wrote:
>
> Adding a dependency to build the Rust compiler even to build one or two
> V4L2 device drivers, would mean a lot to my small hselasky/webcamd
> project. It already has to fetch a copy of the Linux kernel, and now has
> to bootstrap Rust from stage0 to stageN. I personally say no. It's like

Do you mean you need to compile `rustc`? Could you please explain why?
Could you use your distribution's, or fetch the standalone installers
or cache your own toolchain?

> XCode unfortunately. I download 100's of GBytes of upgrades to XCode,
> and barely upload one millionth worth of code back to Apple. It's not
> good. Software developers shouldn't have to download more stuff than
> they upload?

The Rust standalone installers are 2+ orders of magnitude lighter.

> The definition of "bugs" may vary of course. I was thinking more like
> stack exploits, missing validation of arrays and so on.

The kernel definitely needs to avoid those. What do you mean?

> I must admit I'm not a Rust guy and don't see the advantages of Rust
> like you do.

The advantages are fairly clear. The question has always been whether
the cost is worth those benefits.

> Why not move Linux-V4L2 drivers to user-space? In my opinion Rust is
> much more easy to get going there than at the kernel level.

That sounds like an orthogonal discussion.

In any case, please note that you would need to install the same Rust
toolchain to compile them in userspace. So, if you are concerned about
the size of the toolchain (as you mention above), it would not really
make a difference.

> Rust is slow based on my observations building Firefox from sources. The
> Rust compiler spends a significant amount of time per source file.

It is slower than compiling C, but it also provides more features, so
it seems fair for what we are getting in exchange.

Cheers,
Miguel

2023-04-11 00:01:39

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Sun, Apr 9, 2023 at 4:10 PM Daniel Almeida
<[email protected]> wrote:
>
> and I don't
> think there
> are any plans to introduce Rust outside of drivers in the kernel at
> all, last I
> heard.

This was indeed our original proposal (actually, for "leaf" modules in
general, not only device drivers), but where the line is drawn is up
to the kernel maintainers. For instance, some of them have expressed
interest in potentially having Rust subsystems in the future.

Cheers,
Miguel

2023-04-11 08:14:01

by Hans Verkuil

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Daniel,

On 06/04/2023 23:56, Daniel Almeida wrote:
> Hi all, this is my first attempt at adding Rust support to the
> media subsystem.
>
> It adds just enough support to write a clone of the virtio-camera
> prototype written by my colleague, Dmitry Osipenko, available at [0].
>
> Basically, there's support for video_device_register,
> v4l2_device_register and for some ioctls in v4l2_ioctl_ops. There is
> also some initial vb2 support, alongside some wrappers for some types
> found in videodev2.h.
>
> I wrote a sample Rust driver just to prove that this probes, and
> that you get a message on dmesg whenever an ioctl is called.
>
> As there is no actual implementation for any of the ioctls, this module
> can misbehave with some programs. I can work around this in a future
> submission.
>
> Note that this is based on the rust branch, as opposed to rust-next. The
> reasoning is simple: I expect this series to just kickstart some
> discussion around the subject. Actual upstreaming can come up much
> later, at which point I can rebase on the rust branch.
>
> Lastly, the origins of this series trace back to a v4l2 virtIO driver I
> was writing in Rust. As the project was eventually shelved for other
> reasons, I picked both the virtIO and the v4l2 bindings into their own
> patches which I am now in the process of submitting. This is to say that
> I tested this code with said driver and CrosVM in the past, and it
> worked ok.
>
> Please let me know your thoughts.

I think this could be a good topic to discuss at the upcoming media summit:

https://lore.kernel.org/linux-media/[email protected]/

Please reply to that message with a topic description if you are interested.

One of my main concerns here is time: as subsystem maintainers we can barely
keep up with all the incoming patches. Introducing support for a new language
would add only more pressure. Even though these are mainly bindings (as I
understand it), this would still require that every change to a C kAPI is
duplicated in rust, requiring someone to do that work, and have maintainers
with enough rust knowledge to verify it.

And just reading about a new programming language is not enough to be able to
properly review code: that requires far more experience IMHO. The only way
to acquire that experience is by writing non-trivial drivers in rust.

And that in turn takes a lot of time. Which we do not have.

Regards,

Hans

>
> [0]: https://gitlab.collabora.com/dmitry.osipenko/linux-kernel-rd/-/commit/055a2c322e931a8b388f864f1db81bbdfd525602
>
> Daniel Almeida (6):
> rust: media: add the media module
> rust: media: add initial videodev2.h abstractions
> rust: sync: introduce FfiMutex
> rust: media: videobuf2: add a videobuf2 abstraction
> rust: media: add {video|v4l2}_device_register support
> rust: media: add v4l2 rust sample
>
> rust/bindings/bindings_helper.h | 8 +
> rust/kernel/lib.rs | 2 +
> rust/kernel/media/mod.rs | 6 +
> rust/kernel/media/v4l2/capabilities.rs | 80 ++++
> rust/kernel/media/v4l2/dev.rs | 369 +++++++++++++++
> rust/kernel/media/v4l2/device.rs | 115 +++++
> rust/kernel/media/v4l2/enums.rs | 135 ++++++
> rust/kernel/media/v4l2/format.rs | 178 ++++++++
> rust/kernel/media/v4l2/framesize.rs | 176 +++++++
> rust/kernel/media/v4l2/inputs.rs | 104 +++++
> rust/kernel/media/v4l2/ioctls.rs | 608 +++++++++++++++++++++++++
> rust/kernel/media/v4l2/mmap.rs | 81 ++++
> rust/kernel/media/v4l2/mod.rs | 13 +
> rust/kernel/media/videobuf2/core.rs | 552 ++++++++++++++++++++++
> rust/kernel/media/videobuf2/mod.rs | 5 +
> rust/kernel/sync.rs | 1 +
> rust/kernel/sync/ffi_mutex.rs | 70 +++
> samples/rust/Kconfig | 11 +
> samples/rust/Makefile | 1 +
> samples/rust/rust_v4l2.rs | 403 ++++++++++++++++
> 20 files changed, 2918 insertions(+)
> create mode 100644 rust/kernel/media/mod.rs
> create mode 100644 rust/kernel/media/v4l2/capabilities.rs
> create mode 100644 rust/kernel/media/v4l2/dev.rs
> create mode 100644 rust/kernel/media/v4l2/device.rs
> create mode 100644 rust/kernel/media/v4l2/enums.rs
> create mode 100644 rust/kernel/media/v4l2/format.rs
> create mode 100644 rust/kernel/media/v4l2/framesize.rs
> create mode 100644 rust/kernel/media/v4l2/inputs.rs
> create mode 100644 rust/kernel/media/v4l2/ioctls.rs
> create mode 100644 rust/kernel/media/v4l2/mmap.rs
> create mode 100644 rust/kernel/media/v4l2/mod.rs
> create mode 100644 rust/kernel/media/videobuf2/core.rs
> create mode 100644 rust/kernel/media/videobuf2/mod.rs
> create mode 100644 rust/kernel/sync/ffi_mutex.rs
> create mode 100644 samples/rust/rust_v4l2.rs
>

2023-04-11 09:56:54

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 4/11/23 01:41, Miguel Ojeda wrote:
> On Mon, Apr 10, 2023 at 8:59 PM Hans Petter Selasky <[email protected]> wrote:
>>
>> Adding a dependency to build the Rust compiler even to build one or two
>> V4L2 device drivers, would mean a lot to my small hselasky/webcamd
>> project. It already has to fetch a copy of the Linux kernel, and now has
>> to bootstrap Rust from stage0 to stageN. I personally say no. It's like
>
> Do you mean you need to compile `rustc`? Could you please explain why?
> Could you use your distribution's, or fetch the standalone installers
> or cache your own toolchain?

Hi Miguel,

Assume you need to update both the kernel and the rust compiler at the
same time. How do you do that? In the binary download case you have two
machines. One to build rust and one to build the kernel, so it is
technically not possible?

The Rust compiler has a dependency on the kernel and the kernel has a
dependency on the Rust compiler. That just means, some kind of changes
can never happen. This is the ingredient for never ending problems. It's
like you put some rock into the system: If this ever needs to change ...

I'll give you a real-life example to emphasis this. Apple and Microsoft
has done something very bad in the file system area. They mistreat what
happens to be the Norwegian character "å" (0xE5). Norway is where I
live. Their solution is to split the "å" character into the "a"
character (0x61) and the combining ring-over character (0x30A).

There are three problems:

1) Many Unicode implementations only expect one combining ring-over
character. Either this leads directly to a stack exploit or a denial of
service, depending on the actual code: CVE-2023-25193 (ongoing).

2) The proper solution would be to deny this kind of combining
characters, also called umlauts in Germany. Only that requires both
Apple and Microsoft to change and update both their filesystem and
kernel at the same time! The "å" character (0xE5) is essential for
quickly deleting files. Or disable this feature, and rewrite the
directory table every time a file is deleted.

3) Apple and Microsoft managed to screw this up, so that you can create
files under Microsoft (exFat-disk), that don't show up under MacOS. In
iOS they show up however, but can't be copied or moved anywhere. And if
you think your files are backed up in the iCloud, think again!

The consequences can be quite serious, that you could end up being
unfairly judged by the Police in Norway, because court documents "just
got lost" they say.

Do you think Microsoft and Apple will ever change this dependency, if a
change means you need to re-format filesystems live or risk a serious
performance degradation? I have my personal doubts.

I think the problem with the "å" character I've described above, is a
forever problem created by Apple and Microsoft and IBM and who knows
what more. It's not possible to solve, without a serious cost, and
having this secret automagic trashbin for files that just a few people
use, compared to the big picture, is not an issue for them. Even a few
people going to jail for 21 years, is not an issue. Who cares, is the
impression I get from customer support at both Microsoft and Apple. And
not at least, who knows about this really!

Daniel and Miguel: By saying it is not a good thing to build systems
completely from source, both kernel and toolchain and everything that
goes with it, you basically say that permanent "dependencies" between
the compilers and the kernel will never be a problem. You are building
on a rock, and only the future knows if what you consider a rock today
is really a problem tomorrow.

In my example the unicode alphabet is a problem. So tell me: How would
you update a system, if the value of every single letter in the unicode
alphabet would change?

>
>> XCode unfortunately. I download 100's of GBytes of upgrades to XCode,
>> and barely upload one millionth worth of code back to Apple. It's not
>> good. Software developers shouldn't have to download more stuff than
>> they upload?
>
> The Rust standalone installers are 2+ orders of magnitude lighter.

For people that build stuff on their laptops it still matters. If you
have a beefy machine, it is a different case.

>
>> The definition of "bugs" may vary of course. I was thinking more like
>> stack exploits, missing validation of arrays and so on.
>
> The kernel definitely needs to avoid those. What do you mean?

I thought that Rust didn't allow you to write outside the bounds of
arrays, similarly to the old Turbo Pascal language?

>
>> I must admit I'm not a Rust guy and don't see the advantages of Rust
>> like you do.
>
> The advantages are fairly clear. The question has always been whether
> the cost is worth those benefits.

If there could be one base compiler and toolchain, I would be happy.

>
>> Why not move Linux-V4L2 drivers to user-space? In my opinion Rust is
>> much more easy to get going there than at the kernel level.
>
> That sounds like an orthogonal discussion.

Sure.

>
> In any case, please note that you would need to install the same Rust
> toolchain to compile them in userspace. So, if you are concerned about
> the size of the toolchain (as you mention above), it would not really
> make a difference.
>
>> Rust is slow based on my observations building Firefox from sources. The
>> Rust compiler spends a significant amount of time per source file.
>
> It is slower than compiling C, but it also provides more features, so
> it seems fair for what we are getting in exchange.

Right, so think about where that slowness may end up one day, if you
suddenly need to re-build everything from sources so to say :-)

Thanks for your input!

--HPS

2023-04-11 12:11:55

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 9:51 AM Hans Verkuil <[email protected]> wrote:
>
> One of my main concerns here is time: as subsystem maintainers we can barely
> keep up with all the incoming patches. Introducing support for a new language
> would add only more pressure. Even though these are mainly bindings (as I
> understand it), this would still require that every change to a C kAPI is
> duplicated in rust, requiring someone to do that work, and have maintainers
> with enough rust knowledge to verify it.

Indeed, that is one of the main costs.

One potential solution is to have somebody step up as the maintainer
of the Rust side (e.g. the author of the abstractions).

Of course, that will not make the work go to zero, since there still
needs to be some degree of communication even if the new maintainer
does all the Rust side work, but it may make it feasible, especially
if the abstracted parts of the C API do not change too frequently.

It is also an opportunity for existing maintainers to see how the Rust
side would work meanwhile the work gets done, and potentially a chance
to get a new maintainer involved with the whole subsystem in the
future.

Some subsystems may want to give that maintainer a different
`MAINTAINERS` entry, e.g. as a child subsystem that sends PRs to the
main one and may be marked as "experimental". This is also a way to
see how the new abstractions work or not, giving maintainers more time
to decide whether to commit to a Rust side or not.

I don't mean to say it would be doable for the media subsystem, but
please consider it.

Cheers,
Miguel

2023-04-11 12:38:00

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 11:52 AM Hans Petter Selasky <[email protected]> wrote:
>
> Assume you need to update both the kernel and the rust compiler at the
> same time. How do you do that? In the binary download case you have two
> machines. One to build rust and one to build the kernel, so it is
> technically not possible?

I don't understand the problem -- you can build (or download) new
toolchains without changing the kernel, and you can keep several
kernels and several toolchains installed, too.

> I'll give you a real-life example to emphasis this. Apple and Microsoft
> has done something very bad in the file system area. They mistreat what
> happens to be the Norwegian character "å" (0xE5). Norway is where I
> live. Their solution is to split the "å" character into the "a"
> character (0x61) and the combining ring-over character (0x30A).

Sorry, but I don't see how all that relates to the current discussion (kernel).

> Daniel and Miguel: By saying it is not a good thing to build systems
> completely from source,

I haven't said that at all. I regularly build from source myself, in fact.

> For people that build stuff on their laptops it still matters. If you
> have a beefy machine, it is a different case.

I don't follow. You said you are downloading 100s of GiB for XCode,
but you are not OK with 100s of MiBs for Rust?

Anyway, both the Rust toolchain and the kernel can be built on laptops
(I do so), and they don't need to be the highest end ones at all.

> I thought that Rust didn't allow you to write outside the bounds of
> arrays, similarly to the old Turbo Pascal language?

It avoids all UB, including data races, not just out-of-bounds
accesses, as long as the unsafe parts are sound (and there are no
compiler bugs etc.). Which is one of the main reasons we want it in
the kernel.

> If there could be one base compiler and toolchain, I would be happy.

If you mean a single vendor, then it may be interesting for you that
GCC will include Rust support in future releases. It remains to be
seen when their compiler is ready for building the kernel parts, but
it is one of their goals as far as I understand.

> Right, so think about where that slowness may end up one day, if you
> suddenly need to re-build everything from sources so to say :-)

If you want to build everything from source, then you will need some
CPU time to do so. That is just how things work. Most people will just
use the toolchain from their distribution.

> Thanks for your input!

Not at all, thanks for your input too :)

Cheers,
Miguel

2023-04-11 12:57:03

by Willy Tarreau

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Miguel!

On Tue, Apr 11, 2023 at 02:02:17PM +0200, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 9:51?AM Hans Verkuil <[email protected]> wrote:
> >
> > One of my main concerns here is time: as subsystem maintainers we can barely
> > keep up with all the incoming patches. Introducing support for a new language
> > would add only more pressure. Even though these are mainly bindings (as I
> > understand it), this would still require that every change to a C kAPI is
> > duplicated in rust, requiring someone to do that work, and have maintainers
> > with enough rust knowledge to verify it.
>
> Indeed, that is one of the main costs.
>
> One potential solution is to have somebody step up as the maintainer
> of the Rust side (e.g. the author of the abstractions).
>
> Of course, that will not make the work go to zero, since there still
> needs to be some degree of communication even if the new maintainer
> does all the Rust side work, but it may make it feasible, especially
> if the abstracted parts of the C API do not change too frequently.
>
> It is also an opportunity for existing maintainers to see how the Rust
> side would work meanwhile the work gets done, and potentially a chance
> to get a new maintainer involved with the whole subsystem in the
> future.
>
> Some subsystems may want to give that maintainer a different
> `MAINTAINERS` entry, e.g. as a child subsystem that sends PRs to the
> main one and may be marked as "experimental". This is also a way to
> see how the new abstractions work or not, giving maintainers more time
> to decide whether to commit to a Rust side or not.
>
> I don't mean to say it would be doable for the media subsystem, but
> please consider it.

This might sound strange, but I suspect that having a TAINT_RUST flag
could possibly help maintainers that are already lacking time, because
it may quickly allow some of them to ask "please try again without the
Rust code to see if the problem is still there", just like happens with
out-of-tree code for which the knowledge is limited to null. This could
allow to route issue reports to one maintainer when an issue is confirmed
in both cases or to another one when it only happens in a single case.

Of course it will not help with code reviews but we know that a great
part of maintainers' time it spent trying to analyse problem reports
that happen under vague conditions. All the time not spent debugging
something not well understood is more time available for reviews.

Just my two cents,
Willy

2023-04-11 13:16:18

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 4/11/23 14:36, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 11:52 AM Hans Petter Selasky<[email protected]> wrote:
>> Assume you need to update both the kernel and the rust compiler at the
>> same time. How do you do that? In the binary download case you have two
>> machines. One to build rust and one to build the kernel, so it is
>> technically not possible?
> I don't understand the problem -- you can build (or download) new
> toolchains without changing the kernel, and you can keep several
> kernels and several toolchains installed, too.
>

Hi Miguel,

What I'm saying is a bit difficult to get about right off the bat.

>you can build (or download) new toolchains without changing the kernel

Yes, most of the time, but not always. Let me rephrase it:

If you cannot build a new toolchain without a new kernel.

And:

You cannot build a new kernel without a new toolchain.

Then you are stuck forever to build a new toolchain and kernel? Do you
agree?

Or you can say, someone else needs to deal with it, but then you have a
single point of failure.

It's like the next version of the Rust compiler depends on the previous
version of the Rust compiler.

But now you have (Version of Linux and version of Rust) depends on
(Version of Linux and version of Rust).

So in order to upgrade either Linux or Rust, you may be forced to go
through multiple, upgrade kernel, reboot, upgrade rust, build kernel,
reboot and so on.

And that is an annoying complication when upgrading a system.

--HPS

2023-04-11 14:12:18

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hans (Verkuil),

> One potential solution is to have somebody step up as the maintainer
> of the Rust side (e.g. the author of the abstractions).

I'd be willing to step up as maintainer for the Rust V4L2 stuff. I will
also be contributing more bindings if we decide that's worthwhile,
because right now this is obviously very incomplete.

IIRC, Benjamin was originally looking into Rust for the AV1 driver he's
just written in C, but back then there was just too much work to be done
to even get started on the task. My point is that maybe these type of
drivers - i.e.: m2m codec stuff - are good candidates for Rust, because
the codec-specific bits are very self-contained.

-- Daniel

2023-04-11 14:15:24

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 2:49 PM Willy Tarreau <[email protected]> wrote:
>
> This might sound strange, but I suspect that having a TAINT_RUST flag
> could possibly help maintainers that are already lacking time, because
> it may quickly allow some of them to ask "please try again without the
> Rust code to see if the problem is still there", just like happens with
> out-of-tree code for which the knowledge is limited to null. This could
> allow to route issue reports to one maintainer when an issue is confirmed
> in both cases or to another one when it only happens in a single case.
>
> Of course it will not help with code reviews but we know that a great
> part of maintainers' time it spent trying to analyse problem reports
> that happen under vague conditions. All the time not spent debugging
> something not well understood is more time available for reviews.

You can already ask to disable `CONFIG_RUST`.

In fact, we asked that a few times, when people reported a problem
that looked unrelated to Rust, to confirm that was the case and thus
redirect the report.

So it is definitely a good idea to ask for that when you get a report
with `RUST=y` and you suspect it may be related to that, especially in
the beginning where `RUST=y` should not be common.

However, I think Rust in-tree code is different to out-of-tree code,
since you do have the code, and thus (in general) you should be able
to reproduce the build, and you can ask for help to the given
maintainers to understand it.

Cheers,
Miguel

2023-04-11 14:20:28

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 3:15 PM Hans Petter Selasky <[email protected]> wrote:
>
> If you cannot build a new toolchain without a new kernel.

Why not?

> Then you are stuck forever to build a new toolchain and kernel? Do you
> agree?

No, I don't agree, because I don't understand why you cannot build the
new toolchain in the old kernel, or use a pre-built toolchain for that
matter (whether built by you or by somebody else).

> Or you can say, someone else needs to deal with it, but then you have a
> single point of failure.

No, you could build your own toolchain and save it somewhere, if you
don't want to rely on a build from somebody else.

Cheers,
Miguel

2023-04-11 14:30:23

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 12:46 AM Deborah Brouwer
<[email protected]> wrote:
>
> Hi Daniel - I don't know if the 'rust branch' is common knowledge but
> it helped me to know it was this:
> https://github.com/Rust-for-Linux/linux
>
> For what it's worth, I was able to get the V4L2 Rust sample probed
> pretty easily following the quick start instructions
> https://docs.kernel.org/rust/quick-start.html

Thanks, it is great to hear that the guide helped! :)

On resources: nowadays we have a webpage, too. Still to be completed,
but you may find it useful already: https://rust-for-linux.com

Cheers,
Miguel

2023-04-11 15:48:15

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 4/11/23 16:19, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 3:15 PM Hans Petter Selasky <[email protected]> wrote:
>>
>> If you cannot build a new toolchain without a new kernel.
>

Hi,

> Why not?

To me it is very simple:

Look at this:

-#define FE_GET_PROPERTY _IOR('o', 83, struct
dtv_properties)
+#define FE_GET_PROPERTY _IOW('o', 83, struct
dtv_properties)
+#define FE_GET_PROPERTY_OLD _IOR('o', 83, struct dtv_properties)

The FE_GET_PROPERTY IOCTL definition is incorrectly specified as reading
data. While it is actually writing data. When will this be fixed in
Linux - I think never. That's just the way both Linux and GIT works,
unfortunately, though that's another discussion. You can put stuff in,
but you can't easily get stuff out, without it having consequences.

Similarly rustc may depend on an incorrectly specified ioctl()
definition, also via other libraries and static linking, that just have
to stay incorrectly defined, because it was initially incorrectly defined.

Daniel, please explain why the few lines of chunk above (and there are
some more) cannot be upstreamed into Linux?

>
>> Then you are stuck forever to build a new toolchain and kernel? Do you
>> agree?
>
> No, I don't agree, because I don't understand why you cannot build the
> new toolchain in the old kernel, or use a pre-built toolchain for that
> matter (whether built by you or by somebody else).
>
>> Or you can say, someone else needs to deal with it, but then you have a
>> single point of failure.
>
> No, you could build your own toolchain and save it somewhere, if you
> don't want to rely on a build from somebody else.

I'm trying to explain something difficult. And I'm OK that you neither
understand nor agree about my viewpoint. See my replies above.

--HPS

2023-04-11 16:58:42

by Willy Tarreau

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 04:13:36PM +0200, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 2:49?PM Willy Tarreau <[email protected]> wrote:
> >
> > This might sound strange, but I suspect that having a TAINT_RUST flag
> > could possibly help maintainers that are already lacking time, because
> > it may quickly allow some of them to ask "please try again without the
> > Rust code to see if the problem is still there", just like happens with
> > out-of-tree code for which the knowledge is limited to null. This could
> > allow to route issue reports to one maintainer when an issue is confirmed
> > in both cases or to another one when it only happens in a single case.
> >
> > Of course it will not help with code reviews but we know that a great
> > part of maintainers' time it spent trying to analyse problem reports
> > that happen under vague conditions. All the time not spent debugging
> > something not well understood is more time available for reviews.
>
> You can already ask to disable `CONFIG_RUST`.
>
> In fact, we asked that a few times, when people reported a problem
> that looked unrelated to Rust, to confirm that was the case and thus
> redirect the report.
>
> So it is definitely a good idea to ask for that when you get a report
> with `RUST=y` and you suspect it may be related to that, especially in
> the beginning where `RUST=y` should not be common.

But if that code is only under a module, there's no need to turn all
that code off if it's sufficient to be certain the module was no loaded.
Plus it's more friendly to the user who doesn't have to rebuild a kernel,
just blacklist a module and check that the kernel doesn't get tainted
again.

> However, I think Rust in-tree code is different to out-of-tree code,
> since you do have the code, and thus (in general) you should be able
> to reproduce the build, and you can ask for help to the given
> maintainers to understand it.

It could depend on the layer where it plugs and the level of intimacy
with the core. Sometimes you need a deep understanding of all interactions
between elements to imagine possible scenarios.

Cheers,
Willy

2023-04-11 19:25:14

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 5:33 PM Hans Petter Selasky <[email protected]> wrote:
>
> Similarly rustc may depend on an incorrectly specified ioctl()
> definition, also via other libraries and static linking, that just have
> to stay incorrectly defined, because it was initially incorrectly defined.

Why would a compiler depend on random ioctls? Even if it did, how is
that related to the previous discussion? A compiler is just one more
userspace application. Whether the kernel uses C or Rust internally
has nothing to do with that.

Also, I don't follow your logic. You said you cannot upgrade your
toolchain (for some reason), and your argument is that the kernel
keeps interfaces stable? Well, yes, that is the point and what allows
you to upgrade.

Moreover, what is special about `rustc` here? What about your C toolchain?

> I'm trying to explain something difficult. And I'm OK that you neither
> understand nor agree about my viewpoint. See my replies above.

No, it is not a matter of being difficult. It is just that you have
not shown how you would be prevented from upgrading a toolchain.

Cheers,
Miguel

2023-04-11 19:31:36

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 6:52 PM Willy Tarreau <[email protected]> wrote:
>
> But if that code is only under a module, there's no need to turn all
> that code off if it's sufficient to be certain the module was no loaded.
> Plus it's more friendly to the user who doesn't have to rebuild a kernel,
> just blacklist a module and check that the kernel doesn't get tainted
> again.

That could apply to any foreign-to-us subsystems, including C code
too. Should we taint per subsystem so that we can easily check for
those that we may not trust?

I see one could argue for an experimental taint or making it depend on
something like `STAGING`, i.e. based on grounds of being new code. But
I don't see why that should be grounded on just being a different
language or not being able to read the code.

> It could depend on the layer where it plugs and the level of intimacy
> with the core. Sometimes you need a deep understanding of all interactions
> between elements to imagine possible scenarios.

Please note that the policy for submitting new Rust code is that the
respective kernel maintainers and their lists are contacted. We also
request that maintainers take the code through their tree if they can,
rather than going through the Rust tree, precisely so that maintainers
are aware of these potential interactions. See
https://rust-for-linux.com/contributing#the-rust-subsystem for
details.

Cheers,
Miguel

2023-04-11 20:29:07

by Willy Tarreau

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 09:27:35PM +0200, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 6:52?PM Willy Tarreau <[email protected]> wrote:
> >
> > But if that code is only under a module, there's no need to turn all
> > that code off if it's sufficient to be certain the module was no loaded.
> > Plus it's more friendly to the user who doesn't have to rebuild a kernel,
> > just blacklist a module and check that the kernel doesn't get tainted
> > again.
>
> That could apply to any foreign-to-us subsystems, including C code
> too. Should we taint per subsystem so that we can easily check for
> those that we may not trust?

I don't know, maybe that would be a bit too fine. But at least a tainted
flag is much less intrusive than forcing a user to rebuild and disable
possibly important features that they would only be willing to disable
for just a test.

> I see one could argue for an experimental taint or making it depend on
> something like `STAGING`, i.e. based on grounds of being new code.

It could also be an idea.

> But
> I don't see why that should be grounded on just being a different
> language or not being able to read the code.

Because being a different language means some maintainers will always
have a hard time understanding that code that interacts with their
subsystems, even if they try hard. It's exactly the same reason why
25 years ago Linus asked to stop abusing assembly code. If a language
is only understood by a subset of developers, by nature it becomes
more difficult to maintain in some areas.

> > It could depend on the layer where it plugs and the level of intimacy
> > with the core. Sometimes you need a deep understanding of all interactions
> > between elements to imagine possible scenarios.
>
> Please note that the policy for submitting new Rust code is that the
> respective kernel maintainers and their lists are contacted. We also
> request that maintainers take the code through their tree if they can,
> rather than going through the Rust tree, precisely so that maintainers
> are aware of these potential interactions. See
> https://rust-for-linux.com/contributing#the-rust-subsystem for
> details.

Sure, but as you said, "if they can". I thought that it could be both
elegant, lightweight and convenient. But I'm not trying to sell this
idea, just sharing it.

Cheers,
Willy

2023-04-11 22:19:22

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 10:26 PM Willy Tarreau <[email protected]> wrote:
>
> I don't know, maybe that would be a bit too fine. But at least a tainted
> flag is much less intrusive than forcing a user to rebuild and disable
> possibly important features that they would only be willing to disable
> for just a test.

It may be useful early on to have an easy way to check if any Rust
modules got loaded (though note that `RUST` is not tristate so far, so
you would still have something loaded). It could be extra optional
output in e.g. `lsmod`.

However, I don't know why that should imply tainting, especially
medium- and long-term -- please see below.

> have a hard time understanding that code that interacts with their
> subsystems, even if they try hard. It's exactly the same reason why
> 25 years ago Linus asked to stop abusing assembly code. If a language
> is only understood by a subset of developers, by nature it becomes
> more difficult to maintain in some areas.

Yeah, but that is why the idea was that Rust goes first into
subsystems where maintainers are willing to put some time into it now
and evaluate its merits. That way we also build more Rust expertise
across the kernel over time, so that later it is easier for others
(e.g. by having examples of API design and drivers, more people to
refer to, better tooling...).

But, yes, if Rust grows to be really successful within the kernel,
then at some point some basic understanding of Rust will be needed by
most kernel developers. I think that is fine, as long as there is
enough time to adjust.

> Sure, but as you said, "if they can". I thought that it could be both
> elegant, lightweight and convenient. But I'm not trying to sell this
> idea, just sharing it.

To be clear, it is still up to each subsystem to decide whether to
take Rust code. What I meant by "if they can" is that, if they are
willing to, then ideally the code would go through their tree too. The
exception are core APIs, where I asked for flexibility from all sides,
so that those subsystems willing to try Rust do not get completely
blocked.

Cheers,
Miguel

2023-04-12 03:01:25

by Theodore Ts'o

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 04:22:56PM +0200, Miguel Ojeda wrote:
>
> Thanks, it is great to hear that the guide helped! :)
>
> On resources: nowadays we have a webpage, too. Still to be completed,
> but you may find it useful already: https://rust-for-linux.com

Something that would perhaps be useful is to document (a) what
versions of Rust is available for various distributions, or pointers
to how to get that information for various distributions. For
example, you can get that information from Debian using [1]. It
appears that Fedora isn't distributing rustc at *all*, at least
according to [2], so apparently for Fedora people will need to install
it from source.

[1] https://packages.debian.org/search?keywords=rustc&searchon=names&suite=all&section=all
[2] https://idroot.us/install-rust-fedora-37/

The other thing that would be worth documenting is (b) something about
what versions of Rust people have actually tested. The comments at
[3] are quite scary, since per [4], the minimum version of Rustc
supported is 1.62.0 --- and per [3], **only** Rust 1.62.0 is
supported, since we use unstable Rust features.

[3] https://rust-for-linux.com/rust-version-policy
[4] https://docs.kernel.org/process/changes.html

But for example, with Debian, Debian stable is shipping Rust 1.48.0,
and Debian testing (which is currently in "hard freeze" so it can be
released as Debian stable this summer) is shipping Rustc 1.63.0.

Since I use Debian testing, the question which is foremost in my mind
is whether I can expect to have things work if I use the
distro-provided 1.63.0 rustc, or is this really a case of "it's not
Rust 1.62.0, so good luck to you"?

If the goal is accelerate adoption of Rustc, and calm people's fears
vis-a-vis using Rust, it's not enough to say, "why don't you use the
distribution-provided version or Rust"? It would be helpful if those
Rust pioneers can share what versions of Rust they have tested
against, especially for those commonly used distributions, such as
Debian, and give us a report whether we should expect things to work,
so we can ignore the scary warning from the build system that we're
using an unsupported version of Rust, and if it breaks, we get to keep
both pieces.

And for those distributions that don't currently ship Rust, such as
Fedora, if someone could build their own unofficial packages, until we
can convince Red Hat to start shipping -their own supported Rust
compilers, that might be a great way of bridging that gap.

Cheers,

- Ted

2023-04-12 10:07:07

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 4/11/23 21:22, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 5:33 PM Hans Petter Selasky <[email protected]> wrote:
>>
>> Similarly rustc may depend on an incorrectly specified ioctl()
>> definition, also via other libraries and static linking, that just have
>> to stay incorrectly defined, because it was initially incorrectly defined.
>
> Why would a compiler depend on random ioctls? Even if it did, how is
> that related to the previous discussion? A compiler is just one more
> userspace application.

Hi,

Is the right hand knowing what the left hand is doing? Are the people
behind Rust aware Rust is being used for kernel purposes or not?

That's why I brought up the file-system issue with Microsoft and Apple
as an example. The Unicode guys probably knew nothing about what the
letter valued 0xE5 was used for in various file systems, so they thought
it was fine to assign a letter there, the Norwegian "å". I think neither
anyone at the two big companies mentioned tried to stop Unicode from
doing such a clear mistake either.

Microsoft and Apple is the left hand, and Unicode is the right hand.

That's why the toolchain should be included in the Linux kernel. So that
the people using Linux know that the toolchain works as intended when
compiling the Linux kernel.

It's a generic issue. If two organizations that make products for
eachother, don't talk closely together, you risk exactly what I point
at, that some stupid decision will be made by the one party, which
doesn't really affect the other party, but innocent customers infact.

> Why would a compiler depend on random ioctls?

Can you say you can write even a test C-program to multiply two 32-bit
numbers, bit by bit, without even deleting a single character once?
People who say C-programmers never do mistakes, are naive. Even standard
ioctls() may contain mistakes and there needs to be a plan to fix such
issues. And when you think the code is right, the compiler is to blame,
and when you think the compiler is right, the CPU is to blame and so it
goes.

> Whether the kernel uses C or Rust internally
> has nothing to do with that.

The question is not OR, but AND related. If the kernel will need both at
some point in the future, it's not good. The plan should be either OR:
Rustc ^ GCC = true. Not Rustc | GCC = true :-)

> Also, I don't follow your logic. You said you cannot upgrade your
> toolchain (for some reason), and your argument is that the kernel
> keeps interfaces stable? Well, yes, that is the point and what allows
> you to upgrade.

You need to see, stable interfaces may also need to be changed. That is
where you invert my logic. If you fix that when reading my text, you
will see what I'm saying is true and not false.

There may be bit-pattern things down at CPU level, triggering bit-flips,
that CPU vendors will do nothing about, because the argument is
typically about money and performance. If something costs both money and
hurts performance, it will not be implemented. It's like the speculative
instruction prediction and resulting cache pollution, allowing memory to
leak from kernel level to user-space level. Isn't it enough to deal with
this in GCC only? Does Rust handle such issues at all? I don't know simply.

And what about syscall numbers? What if someone from Intel says all
syscall numbers must be divisible by four, because those two lower
bit-lines are frequently subject to bit flips and we can do nothing
about it.

>
> Moreover, what is special about `rustc` here? What about your C toolchain?

I don't know Rustc that well, so I cannot answer what's special about
it. But based on my existing experience with C toolchains, I don't
expect it to be any easier, with regards to handling unforeseen issues.

>
>> I'm trying to explain something difficult. And I'm OK that you neither
>> understand nor agree about my viewpoint. See my replies above.
>
> No, it is not a matter of being difficult. It is just that you have
> not shown how you would be prevented from upgrading a toolchain.

The proof is in a principle. Principles are there to avoid unpredictable
problems.

Apparently you don't accept the principle of talking closely together
when you are in a supply chain.

I have a feeling you think like this: If I do my job great, and all
others in the supply chain do their best, then the resulting product
will be the great too!

Translated to your case: Linux is the most stable OS in the world, and
Rust is the most secure compiler language in the world. Nothing can go
wrong!

--HPS

>
> Cheers,
> Miguel

2023-04-12 10:15:11

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Wed, Apr 12, 2023 at 12:00:59PM +0200, Hans Petter Selasky wrote:
> That's why the toolchain should be included in the Linux kernel. So that the
> people using Linux know that the toolchain works as intended when compiling
> the Linux kernel.

That's not how Linux has ever worked, sorry. So this is not even a
valid discussion anymore.

greg k-h

2023-04-12 10:37:19

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 4/12/23 12:13, Greg KH wrote:
> On Wed, Apr 12, 2023 at 12:00:59PM +0200, Hans Petter Selasky wrote:
>> That's why the toolchain should be included in the Linux kernel. So that the
>> people using Linux know that the toolchain works as intended when compiling
>> the Linux kernel.
>
> That's not how Linux has ever worked, sorry. So this is not even a
> valid discussion anymore.
>

Well, maybe it's time to change your views on that and stop being a rock
thrower on your friends, the compiler folks, whenever something goes wrong:

https://news.ycombinator.com/item?id=8089321

--HPS

2023-04-12 12:29:33

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Wed, Apr 12, 2023 at 4:58 AM Theodore Ts'o <[email protected]> wrote:
>
> Something that would perhaps be useful is to document (a) what
> versions of Rust is available for various distributions, or pointers
> to how to get that information for various distributions. For
> example, you can get that information from Debian using [1]. It
> appears that Fedora isn't distributing rustc at *all*, at least
> according to [2], so apparently for Fedora people will need to install
> it from source.

As far as I understand, Fedora is actually one of the distributions
that provide a very up-to-date Rust toolchain (the latest) and can be
installed via `dnf` [1][2].

Cc'ing Josh Stone who maintains the toolchain in Fedora, just in case
there is something I am missing that the webpage may be referring to.

[1] https://packages.fedoraproject.org/pkgs/rust/rust/
[2] https://developer.fedoraproject.org/tech/languages/rust/rust-installation.html

> The other thing that would be worth documenting is (b) something about
> what versions of Rust people have actually tested. The comments at
> [3] are quite scary, since per [4], the minimum version of Rustc
> supported is 1.62.0 --- and per [3], **only** Rust 1.62.0 is
> supported, since we use unstable Rust features.

The one that we test, for the moment, is the minimum one (since it is
the "only" one) -- I will make it more clear in the webpage.

> But for example, with Debian, Debian stable is shipping Rust 1.48.0,
> and Debian testing (which is currently in "hard freeze" so it can be
> released as Debian stable this summer) is shipping Rustc 1.63.0.
>
> Since I use Debian testing, the question which is foremost in my mind
> is whether I can expect to have things work if I use the
> distro-provided 1.63.0 rustc, or is this really a case of "it's not
> Rust 1.62.0, so good luck to you"?

Distro versions should be fine (as in: it is not an issue of "official
prebuilt images" vs. "distro binaries"). But short-term you likely
need to match the numbered version (or perform small changes in the
kernel side when needed). So, in practice, the easiest route is to use
the binaries provided by Rust itself (via `rustup` or standalone
installers). We could also provide them at kernel.org like for other
toolchains if that helps.

So if distributions want to start using Rust code in their kernels
right now (i.e. before we can declare a proper minimum) with their own
`rustc` package, then one approach they can use is to provide an extra
`rustc-kernel` package matching the version required by the kernel (or
to change the kernel side a bit to make it compile).

We could, in principle, attempt to support several versions in the
kernel side, but given the upstreaming of the first Rust modules is
still going on (and the abstractions they depend on), we still have
some time. Moreover, the first Rust modules may not be needed by most
distributions: what is being upstreamed is the Android Binder driver,
the Asahi GPU driver and the NVMe driver so far.

So it is up to distributions to decide whether they need to use one of
those modules early on, and if that is the case and they need to do so
before we can declare a minimum, then I think it is reasonable to ask
them to match the version. Some particular users, e.g. Android, as far
as I understand, they are OK with matching the version for the time
being.

In summary, the issue is that the minimum version we will eventually
support is "in the future".

> If the goal is accelerate adoption of Rustc, and calm people's fears
> vis-a-vis using Rust, it's not enough to say, "why don't you use the
> distribution-provided version or Rust"? It would be helpful if those
> Rust pioneers can share what versions of Rust they have tested
> against, especially for those commonly used distributions, such as
> Debian, and give us a report whether we should expect things to work,
> so we can ignore the scary warning from the build system that we're
> using an unsupported version of Rust, and if it breaks, we get to keep
> both pieces.

Definitely -- we should be testing distro-versions in CI as soon as it
is (reasonably) possible. We could even do so already for those
distributions that track the latest version, but then it means we will
be upgrading more often (every 6 weeks) in the kernel side. There
would still be small windows where it may not work depending on how
the schedules match, though (but distros could keep a `rustc-old` for
some days until it matches again, for instance).

> Fedora, if someone could build their own unofficial packages, until we
> can convince Red Hat to start shipping -their own supported Rust
> compilers, that might be a great way of bridging that gap.

I think all major distributions already ship Rust. From a quick look:
Fedora (1.68.2), Gentoo (1.68.2), Arch (1.68.2), openSUSE (1.68.0 for
the rolling one), Ubuntu (1.67.1 for 23.04), Debian (1.63.0 this
summer), Red Hat (1.62.1 in 9.1)...

As mentioned above, if kernel maintainers are happy with more frequent
upgrades (i.e. tracking the latest release), it would at least be
easier for those in distributions that track the latest Rust release
-- I would like to do that, in fact, if nobody opposes, since our idea
is to anyhow upgrade the compiler required in the kernel until we hit
the minimum.

What do you think?

Cheers,
Miguel

2023-04-12 14:37:41

by Morten Linderud

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 10:58:34PM -0400, Theodore Ts'o wrote:
> On Tue, Apr 11, 2023 at 04:22:56PM +0200, Miguel Ojeda wrote:
> >
> > Thanks, it is great to hear that the guide helped! :)
> >
> > On resources: nowadays we have a webpage, too. Still to be completed,
> > but you may find it useful already: https://rust-for-linux.com
>
> Something that would perhaps be useful is to document (a) what
> versions of Rust is available for various distributions, or pointers
> to how to get that information for various distributions. For
> example, you can get that information from Debian using [1]. It
> appears that Fedora isn't distributing rustc at *all*, at least
> according to [2], so apparently for Fedora people will need to install
> it from source.

You can get a list here:
https://repology.org/project/rust/versions

Another alternative is this webpage:
https://pkgs.org/download/rust

--
Morten Linderud
PGP: 9C02FF419FECBE16

Attachments:

(No filename) (970.00 B)
signature.asc (849.00 B)
Download all attachments

2023-04-12 18:50:26

by Nicolas Dufresne

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Theodore,

Le mardi 11 avril 2023 à 22:58 -0400, Theodore Ts'o a écrit :
> And for those distributions that don't currently ship Rust, such as
> Fedora, if someone could build their own unofficial packages, until we
> can convince Red Hat to start shipping -their own supported Rust
> compilers, that might be a great way of bridging that gap.

Rust can be installed from package on Fedora. I sense a lot of unverified
supposition to justify your argument. I don't believe this contribute much to
the discussion. It takes about 30s to search on your preferred search engine and
find the fact the Fedora ships rustc, and the version is very recent.

regards,
Nicolas

2023-04-26 00:48:43

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Tue, Apr 11, 2023 at 02:02:17PM +0200, Miguel Ojeda wrote:
> On Tue, Apr 11, 2023 at 9:51 AM Hans Verkuil <[email protected]> wrote:
> >
> > One of my main concerns here is time: as subsystem maintainers we can barely
> > keep up with all the incoming patches. Introducing support for a new language
> > would add only more pressure. Even though these are mainly bindings (as I
> > understand it), this would still require that every change to a C kAPI is
> > duplicated in rust, requiring someone to do that work, and have maintainers
> > with enough rust knowledge to verify it.

Another issue is that the V4L2 subsystem is plagued with lifetime
management problems. I don't think rust bindings could be safely written
for the MC and V4L2 subdev in-kernel APIs at the moment for instance.
Sakari recently attempted to fix some of those issues (again), see [1].
Progress is slow on this front because V4L2 is generally understaffed.

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

Now, I hope that mentioning "lifetime management problems" will be
enough to nerd-snipe a rust enthusiast or two to help fix the C code in
order to implement proper rust bindings on top ;-)

> Indeed, that is one of the main costs.
>
> One potential solution is to have somebody step up as the maintainer
> of the Rust side (e.g. the author of the abstractions).

That would certainly be a required step, but I don't think it would be
enough. On good days I see the media subsystem as barely able to cope
with the current load, on bad days it feels it's completely collapsing.

We have homework to do when it comes to maintenance for the media
subsystem, we're doing *really* badly at the moment regarding community
management and attracting (and retaining) new core contributors. This is
a topic I really want to discuss face to face during the media workshop
in Prague (and I know that many people are looking forward to that
discussion).

> Of course, that will not make the work go to zero, since there still
> needs to be some degree of communication even if the new maintainer
> does all the Rust side work, but it may make it feasible, especially
> if the abstracted parts of the C API do not change too frequently.
>
> It is also an opportunity for existing maintainers to see how the Rust
> side would work meanwhile the work gets done, and potentially a chance
> to get a new maintainer involved with the whole subsystem in the
> future.
>
> Some subsystems may want to give that maintainer a different
> `MAINTAINERS` entry, e.g. as a child subsystem that sends PRs to the
> main one and may be marked as "experimental". This is also a way to
> see how the new abstractions work or not, giving maintainers more time
> to decide whether to commit to a Rust side or not.
>
> I don't mean to say it would be doable for the media subsystem, but
> please consider it.

--
Regards,

Laurent Pinchart

2023-04-26 13:16:17

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 12.04.23 00:14, Miguel Ojeda wrote:

> But, yes, if Rust grows to be really successful within the kernel,
> then at some point some basic understanding of Rust will be needed by
> most kernel developers. I think that is fine, as long as there is
> enough time to adjust.

The tricky question is: how much time will be needed ?

Personally, I'm too overloaded for diving deeper into Rust anytime soon.

I've recently managed giving up my reluctance against golang and doing
some fun project w/ it (freecity, a simcity2000 clone), just to get some
real hands-on experience (besides some smaller patches for other
projects i've done over the years).

Rust and golang share some common problems (when coming from traditional
C + friends):
* entirely different toolchain concept (workflows are very different
from what one's used from GCC + friends)
* fast-moving target (one has to be careful to expect/use the right
toolchain version)
* rarely understood by traditional kernel devs
* distro/build engine integration/support still pretty infant,
especially in embedded world (very related to the toolchain update
problem)

IMHO, before we can practically use Rust at greater scale in the kernel,
the problems above need to be resolved first. And that's something that
the Rust community (not the kernel community) should take care of.

And beware: demanding newer toolchains (thus newer distros), just for
building the kernel, can easily cause *huge* trouble many organisations,
especially in embedded field. Linux is used in lots of highly safety
critical environments that need special verification processes and so
cannot easily upgrade toolchains. If Linux some day suddenly requires
another language like Rust, those would be immediately cut-off from
newer releases.

Ergo: the whole process of adding Rust to the Kernel needs to be done
very, very carefully.

> To be clear, it is still up to each subsystem to decide whether to
> take Rust code. What I meant by "if they can" is that, if they are
> willing to, then ideally the code would go through their tree too. The
> exception are core APIs, where I asked for flexibility from all sides,
> so that those subsystems willing to try Rust do not get completely > blocked.

For the reasons above, the subsystems shouldn't take those decisions
lightly, even if they happen to be Rust experts - this could have a
dramatic effect on downstreams.

Maybe we should (for certain time) go a different path: move all new
Rust stuff (except for bugfixes) to a separate downstream tree, that's
rebased on mainline releases, but still let the patches fload through
the corresponding subsystems.

--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2023-04-26 13:16:47

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 26.04.23 02:32, Laurent Pinchart wrote:

> We have homework to do when it comes to maintenance for the media
> subsystem, we're doing *really* badly at the moment regarding community
> management and attracting (and retaining) new core contributors.

Is this a problem of the subsys (core) itself or individual drivers ?

--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2023-04-26 13:34:12

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 12.04.23 04:58, Theodore Ts'o wrote:

> Something that would perhaps be useful is to document (a) what
> versions of Rust is available for various distributions, or pointers
> to how to get that information for various distributions. For
> example, you can get that information from Debian using [1]. It
> appears that Fedora isn't distributing rustc at *all*, at least
> according to [2], so apparently for Fedora people will need to install
> it from source.

As already said in my other mail, one major problem IMHO is (recent
enough) toolchain availability for the major distros and package build
systems - including the embedded ones (ptxdist, buildroot, bitbake,
...).

IMHO, those who want Rust in the kernel, should take care of this first.
(and no: asking to download some precompiled binary from somewhere is
not any acceptable solution)

> If the goal is accelerate adoption of Rustc, and calm people's fears
> vis-a-vis using Rust, it's not enough to say, "why don't you use the
> distribution-provided version or Rust"? It would be helpful if those
> Rust pioneers can share what versions of Rust they have tested
> against, especially for those commonly used distributions, such as
> Debian, and give us a report whether we should expect things to work,
> so we can ignore the scary warning from the build system that we're
> using an unsupported version of Rust, and if it breaks, we get to keep
> both pieces.

ACK. Maybe those folks could set up some CIs for at least building and
deploying the Rust patches on as many distros as possible - hopefully
before they're sent to lkml.

--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2023-04-26 13:39:43

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Wed, Apr 26, 2023 at 03:13:10PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 26.04.23 02:32, Laurent Pinchart wrote:
>
> > We have homework to do when it comes to maintenance for the media
> > subsystem, we're doing *really* badly at the moment regarding community
> > management and attracting (and retaining) new core contributors.
>
> Is this a problem of the subsys (core) itself or individual drivers ?

It's first and foremost a subsystem core issue. Drivers will also have
to be fixed, but the core needs to be handled first.

--
Regards,

Laurent Pinchart

2023-04-26 14:50:42

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 08.04.23 21:43, Hans Petter Selasky wrote:

> You assume that all
> code is running inside the kernel and needs to be perfect.

Yes, of course. It's kernel code.

If you're borrowing kernel code for your userland stuff, than it's up
to you to take care of it.

There is no such thing like stable/fixed in-kernel APIs - it always has
been this way.

> But I think
> you could just aswell implement the next USB webcam V4L2 driver in Perl
> for that sake.

That would't be v4l anymore.

> The reason for my point of view, is that I think most of the drivers in
> media/ should run in user-space, and not inside the kernel.

Feel free to provide fully userspace drivers for those devices, but
that's totally unrelated to kernel development (no matter whether you're
copying over kernel code into your userland project).

> The example of secure V4L2 programming is already here:
> https://github.com/hselasky/webcamd

BSD code is not in scope of the LKML.

> I would rather like more drive on that, than flowing down the Rust
> stream.

Orthogonal topic.

> Rust is cool, Java is cool, VM's are cool. The only bad about
> cool things, is that they are so slow. For many years I completely
> avoided C++ code for the sake it is very slow to compile, compared to
> bare C code. And when looking at how Firefox is building using Rust, I
> am a little worried, why we need so much code in there!

Yes, compiling Rust is slow, compared to C. That's the price for
sophisticated optimizations. How often do you have to do a full
recompile ?

> Engineering energy would be much more focused, if hardware vendors could
> agree more about what binary formats to use for their device protocols,

Indeed. But we (kernel folks) have no influence on that. If we could,
we'd already standardized HW interfaces for lots of things and so only
a small percentage of the drivers we currently have, while still
supporting the same number of HW (or even more). But unfortunately thats
not under our control. Therefore offtopic.

> than changing the coding language, so that now anyone can be let loose
> to program in the Linux kernel without risking any damage.

AFAIK, this discussion isn't about changing the kernel's programming
language, but just adding language bindings, so some new drivers could
be written in that language. If this really happens and you really want
a C implementation, feel free to send patches.

> The goal for Linux driver development should be fewer drivers and not
> more.

Depends on specific case. We already have lots of drivers that support
wide range of devices. But it doesn't make sense having monster drivers
for entirely different devices.

> I don't want Linux to become the next Microsoft, with gigabytes
> of drivers which are never used for anything.

Actually, we're doing a pretty good job of generalizing things that can
be generalized (if you find room for more improvements, feel free to
send patches). Nobody here seriously intents dropping the subsystem and
lib architectures in favour of monolithic monster drivers like in
Windows world.

> The webcamd daemon already is close to 6 MBytes big on amd64 on FreeBSD.
> Inside there is support for 510 drivers (counting =y keywords), built
> straight off Linus Torvalds:

You have ~500 drivers in one 6MB binary and blame us for writing too
much code ? Maybe you should think about modularization (which we do
have in the kernel).

And, btw, FreeBSD is completely off-topic here.

> The USB video class is great, instead of tons of GSPCA devices, then
> yeah, we don't need to drag around so much legacy binaries, just to make
> everyone happy. What did Apple do? Custom PCI webcam devices? Why can't
> they just stick with virtual USB devices, and then have a dual
> configured device, one config for their own HD codec, and one config for
> people like me, just needing the framebuffer.

Well, they just could have an USB device layered on-top of a tiny PCI-
USB bridge. We're the wrong to blame - talk to Apple.

--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2023-04-26 14:55:25

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 10.04.23 20:59, Hans Petter Selasky wrote:

> Adding a dependency to build the Rust compiler even to build one or two
> V4L2 device drivers, would mean a lot to my small hselasky/webcamd
> project. It already has to fetch a copy of the Linux kernel, and now has
> to bootstrap Rust from stage0 to stageN.

Did it ever cross your mind, being a bit thankful that you can use our
Linux code for your BSD project ?

Linux kernel code was never meant to be used anywhere outside the Linux
kernel. It's cool to see that such things are possible, but that's
really out of scope here.

> I personally say no. It's like
> XCode unfortunately. I download 100's of GBytes of upgrades to XCode,
> and barely upload one millionth worth of code back to Apple. It's not
> good. Software developers shouldn't have to download more stuff than
> they upload?

Where does the connection to Xcode come from ?

> Why not move Linux-V4L2 drivers to user-space?

Feel free to send patches.

>> The reality is that it isn't up to anyone to say who should or
>> shouldn't become
>> a kernel developer. The resources are out there for anyone to try, and
>> if
>> maintainers take in their patches, then that's the end of the story.
> The GPLv2 license should not be the only reason behind Linux developers
> putting drivers in the kernel-space. I think moving more stuff to
> user-space would benefit a greater purpose.

I don't recall any specific case for the license being the primary
reason for putting someting into the kernel instead of userland.

If you want userland v4l drivers: feel free to send patches. Those
should also be capable of directly passing around HW buffers between
separate devices (eg. fg -> codec -> display), which often is
performance critical in embedded devices.

--mtx

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2023-04-26 15:41:06

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Wed, Apr 26, 2023 at 3:37 PM Enrico Weigelt, metux IT consult
<[email protected]> wrote:
>
> As already said in my other mail, one major problem IMHO is (recent
> enough) toolchain availability for the major distros and package build
> systems - including the embedded ones (ptxdist, buildroot, bitbake,
> ...).
>
> IMHO, those who want Rust in the kernel, should take care of this first.
> (and no: asking to download some precompiled binary from somewhere is
> not any acceptable solution)

Some distributions already provide up-to-date Rust versions compiled
by themselves. Those should work if we start tracking the latest
version, which is what I discussed above.

You can, of course, do it yourself too and build the compiler
yourself. Other that that, if you are not OK with third-party
binaries, or binaries we could potentially upload to kernel.org, there
is little we can do until the minimum version arrives to your favorite
distribution.

Having said that, we still need to declare a minimum version, and for
that, extra funding or engineer-hours would be helpful. If your
organization/company is up for it, please contact me.

> ACK. Maybe those folks could set up some CIs for at least building and
> deploying the Rust patches on as many distros as possible - hopefully
> before they're sent to lkml.

I am unsure what you are asking for. Testing patches for the Rust
subsystem? We already do that, of course, and some independent CIs and
companies have already started building with Rust some configs (e.g.
KernelCI and 0-Day).

If you are concerned about distributions breaking their toolchains,
well, sure, we could also test their packages. But it would be best
that, instead, distributions looking to support kernel developers set
up a test on their side, since they are the ones deciding what to do
with their toolchain packages.

I talked with a few distributions' maintainers about this lately, and
most of them were very helpful, so they may be interested in
supporting kernel developers using their distribution for development.
Would that help?

Cheers,
Miguel

2023-04-26 16:12:20

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Wed, Apr 26, 2023 at 3:36 PM Enrico Weigelt, metux IT consult
<[email protected]> wrote:
>
> The tricky question is: how much time will be needed ?

That depends on how fast Rust grows in the kernel itself, so I would
expect it will be a self-regulating feedback loop.

> Personally, I'm too overloaded for diving deeper into Rust anytime soon.

That is fine, one can always wait a bit more to see how things evolve.

Now, if somebody wants to use Rust in a subsystem you maintain, then
you can always consider letting them maintain that part. As I
mentioned above, that can be a quite nice approach to learn Rust on
the side and recruit new future maintainers/reviewers.

> Rust and golang share some common problems (when coming from traditional
> C + friends):
> * entirely different toolchain concept (workflows are very different
> from what one's used from GCC + friends)

I am not sure what you mean, but `rustc` uses LLVM for codegen, and
`rustc_codegen_gcc` and GCC Rust are coming.

If you mean on the developer UX side, for the kernel at least, you
still call `make` as usual. Of course, some things differ here and
there, and there are still things to improve, but it is fairly usable
even at this stage.

We are also working on introducing some features that the C side does
not have yet. So there can be upsides on this regard, too.

> * fast-moving target (one has to be careful to expect/use the right
> toolchain version)

This currently applies to the kernel, yes, because we require some
unstable features.

To be clear, this is something that we are working on solving,
precisely because we know it is not ideal. In any case, the decision
made was to go forward meanwhile that got solved, and it is not a
blocker for some users/companies.

> * rarely understood by traditional kernel devs

Not sure what you mean by this, but a few traditional kernel devs have
successfully picked up Rust already and are interested in increasing
their usage of it.

> * distro/build engine integration/support still pretty infant,

Most distros package Rust as explained above, which may be enough for
you depending on the distro you use.

> IMHO, before we can practically use Rust at greater scale in the kernel,
> the problems above need to be resolved first. And that's something that

That depends on the user/company/entity. For instance, some companies
and projects already want to use (or are using) Rust in the kernel,
because they control their toolchains.

> And beware: demanding newer toolchains (thus newer distros), just for
> building the kernel, can easily cause *huge* trouble many organisations,
> especially in embedded field. Linux is used in lots of highly safety
> critical environments that need special verification processes and so
> cannot easily upgrade toolchains. If Linux some day suddenly requires
> another language like Rust, those would be immediately cut-off from
> newer releases.

That is fine -- many companies and projects have different
requirements, and nobody expects everybody to enable Rust in their
kernel nor to make it a hard/unconditional requirement anytime soon
(if it ever happens).

> Ergo: the whole process of adding Rust to the Kernel needs to be done
> very, very carefully.

Indeed -- if you have particular concerns that you think have not been
addressed yet in the last 2+ years, please contact us.

> For the reasons above, the subsystems shouldn't take those decisions
> lightly, even if they happen to be Rust experts - this could have a
> dramatic effect on downstreams.

There is no effect on downstream, unless they drop support for what
they already have. But that is just like any other proposed removal.

> Maybe we should (for certain time) go a different path: move all new
> Rust stuff (except for bugfixes) to a separate downstream tree, that's
> rebased on mainline releases, but still let the patches fload through
> the corresponding subsystems.

That would not accomplish anything except making everything more
opaque, not to mention harder for downstream users who are waiting for
things to land.

Again, if you have particular concerns, please feel free to raise
them, but please note that most of this has been discussed for a long
time and decided upon.

Cheers,
Miguel

2023-04-26 16:20:41

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Wed, Apr 26, 2023 at 2:32 AM Laurent Pinchart
<[email protected]> wrote:
>
> Now, I hope that mentioning "lifetime management problems" will be
> enough to nerd-snipe a rust enthusiast or two to help fix the C code in
> order to implement proper rust bindings on top ;-)

Good idea ;)

I think it is definitely a good opportunity to consider how Rust could
fit the new design, and perhaps borrow some ideas from Rust for the
new design, even. If you feel like a quick meeting could help on that,
please let us know.

> That would certainly be a required step, but I don't think it would be
> enough. On good days I see the media subsystem as barely able to cope
> with the current load, on bad days it feels it's completely collapsing.
>
> We have homework to do when it comes to maintenance for the media
> subsystem, we're doing *really* badly at the moment regarding community
> management and attracting (and retaining) new core contributors. This is
> a topic I really want to discuss face to face during the media workshop
> in Prague (and I know that many people are looking forward to that
> discussion).

I am sorry to hear that. One idea would be offsetting the extra work
by having the Rust person also take care of some of the C parts too.
That way you can also potentially get them to be a full maintainer at
some point, even if the Rust experiment does not pan out.

Of course, easier said than done, and managing more people always
takes extra time, but getting more people seems to be part of the
solution anyway, from what you say.

In any case, thanks a lot for at least considering it :)

Cheers,
Miguel

2023-04-26 16:44:54

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Miguel,

(CC'ing Sakari Ailus)

On Wed, Apr 26, 2023 at 06:18:35PM +0200, Miguel Ojeda wrote:
> On Wed, Apr 26, 2023 at 2:32 AM Laurent Pinchart wrote:
> >
> > Now, I hope that mentioning "lifetime management problems" will be
> > enough to nerd-snipe a rust enthusiast or two to help fix the C code in
> > order to implement proper rust bindings on top ;-)
>
> Good idea ;)
>
> I think it is definitely a good opportunity to consider how Rust could
> fit the new design, and perhaps borrow some ideas from Rust for the
> new design, even. If you feel like a quick meeting could help on that,
> please let us know.

I think we have a fairly good view of what needs to be done, the rules
are the same regardless of the programming language and whether the
compiler or reviewers enforce them (Hans, Sakari, please feel free to
disagree). Thanks for your offer though, it's appreciated.

> > That would certainly be a required step, but I don't think it would be
> > enough. On good days I see the media subsystem as barely able to cope
> > with the current load, on bad days it feels it's completely collapsing.
> >
> > We have homework to do when it comes to maintenance for the media
> > subsystem, we're doing *really* badly at the moment regarding community
> > management and attracting (and retaining) new core contributors. This is
> > a topic I really want to discuss face to face during the media workshop
> > in Prague (and I know that many people are looking forward to that
> > discussion).
>
> I am sorry to hear that. One idea would be offsetting the extra work
> by having the Rust person also take care of some of the C parts too.
> That way you can also potentially get them to be a full maintainer at
> some point, even if the Rust experiment does not pan out.

That's certainly something I would consider very positive. If anyone is
interested in having a look at (part of) the problem and possible
solutions, [1] is the most recent patch series posted to handle some of
the lifetime issues, and [2] is a more generic version of part of [1].

[1] https://lore.kernel.org/linux-media/[email protected]/
[2] https://lore.kernel.org/linux-kernel/161117153248.2853729.2452425259045172318.stgit@dwillia2-desk3.amr.corp.intel.com/

> Of course, easier said than done, and managing more people always
> takes extra time, but getting more people seems to be part of the
> solution anyway, from what you say.
>
> In any case, thanks a lot for at least considering it :)

--
Regards,

Laurent Pinchart

2023-04-26 17:18:33

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi,

As I said higher up on this thread, I can maintain the Rust bits and
help out with the issues around it.

IMHO, we should at least try this. Who knows, it might work out :)

Laurent, maybe we can take a piecemeal approach? Right now there are no
bindings for MC, but I wouldn't complain about fixing some of the C code
when the time comes.

Just FYI, I am writing some more bindings, just enough to write a
stateless decoder driver. I hope to finish it in time for the media
summit. It will give us a more in-depth idea of the pros and cons here.

-- Daniel

2023-04-26 17:30:35

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Daniel,

On Wed, Apr 26, 2023 at 06:14:33PM +0100, Daniel Almeida wrote:
> Hi,
>
> As I said higher up on this thread, I can maintain the Rust bits and
> help out with the issues around it.
>
> IMHO, we should at least try this. Who knows, it might work out :)
>
> Laurent, maybe we can take a piecemeal approach? Right now there are no
> bindings for MC, but I wouldn't complain about fixing some of the C code
> when the time comes.

The lifetime issues affect plain V4L2 video nodes too I'm afraid :-)

> Just FYI, I am writing some more bindings, just enough to write a
> stateless decoder driver. I hope to finish it in time for the media
> summit. It will give us a more in-depth idea of the pros and cons here.

--
Regards,

Laurent Pinchart

2023-04-26 20:07:59

by Sakari Ailus

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Hi Laurent, Miguel,

On Wed, Apr 26, 2023 at 07:35:12PM +0300, Laurent Pinchart wrote:
> Hi Miguel,
>
> (CC'ing Sakari Ailus)

Thanks for cc'ing me.

>
> On Wed, Apr 26, 2023 at 06:18:35PM +0200, Miguel Ojeda wrote:
> > On Wed, Apr 26, 2023 at 2:32 AM Laurent Pinchart wrote:
> > >
> > > Now, I hope that mentioning "lifetime management problems" will be
> > > enough to nerd-snipe a rust enthusiast or two to help fix the C code in
> > > order to implement proper rust bindings on top ;-)
> >
> > Good idea ;)
> >
> > I think it is definitely a good opportunity to consider how Rust could
> > fit the new design, and perhaps borrow some ideas from Rust for the
> > new design, even. If you feel like a quick meeting could help on that,
> > please let us know.
>
> I think we have a fairly good view of what needs to be done, the rules
> are the same regardless of the programming language and whether the
> compiler or reviewers enforce them (Hans, Sakari, please feel free to
> disagree). Thanks for your offer though, it's appreciated.

I guess on many you don't need to care about lifetime management if you can
assume that certain things never go away. Sometimes these assumptions prove
incorrect, and that's what's happened here.

>
> > > That would certainly be a required step, but I don't think it would be
> > > enough. On good days I see the media subsystem as barely able to cope
> > > with the current load, on bad days it feels it's completely collapsing.
> > >
> > > We have homework to do when it comes to maintenance for the media
> > > subsystem, we're doing *really* badly at the moment regarding community
> > > management and attracting (and retaining) new core contributors. This is
> > > a topic I really want to discuss face to face during the media workshop
> > > in Prague (and I know that many people are looking forward to that
> > > discussion).
> >
> > I am sorry to hear that. One idea would be offsetting the extra work
> > by having the Rust person also take care of some of the C parts too.
> > That way you can also potentially get them to be a full maintainer at
> > some point, even if the Rust experiment does not pan out.
>
> That's certainly something I would consider very positive. If anyone is
> interested in having a look at (part of) the problem and possible
> solutions, [1] is the most recent patch series posted to handle some of
> the lifetime issues, and [2] is a more generic version of part of [1].
>
> [1] https://lore.kernel.org/linux-media/[email protected]/
> [2] https://lore.kernel.org/linux-kernel/161117153248.2853729.2452425259045172318.stgit@dwillia2-desk3.amr.corp.intel.com/

Thanks for the pointer, this would be nice indeed.

I haven't had time to work on the media device referencing series, overall
it should be good for merging but Hans found an issue I haven't had time to
debug yet. I do intend to continue that in the near future though.

--
Regards,

Sakari Ailus

2023-05-01 20:22:51

by Nicolas Dufresne

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Le mercredi 26 avril 2023 à 20:25 +0300, Laurent Pinchart a écrit :
> Hi Daniel,
>
> On Wed, Apr 26, 2023 at 06:14:33PM +0100, Daniel Almeida wrote:
> > Hi,
> >
> > As I said higher up on this thread, I can maintain the Rust bits and
> > help out with the issues around it.
> >
> > IMHO, we should at least try this. Who knows, it might work out :)
> >
> > Laurent, maybe we can take a piecemeal approach? Right now there are no
> > bindings for MC, but I wouldn't complain about fixing some of the C code
> > when the time comes.
>
> The lifetime issues affect plain V4L2 video nodes too I'm afraid :-)

Everything under the bindings is unsafe code, so it does not prevent doing upper
implementation and have other things be memory safe. It just make Rust less
helpful in some cases (I guess everything across ops).

There is low hanging fruit if some folks are interested. I see legitimate
benefit in rewriting in rust the JPEG parser, the H.264 reference list
generator, and maybe VP9 probability update lib. AV1 driver will need a lib to
reduce duplicates, this could be done straight in Rust (offering a C interface
of course, so it does not matter if the users are written in rust or C).

Nicolas

>
> > Just FYI, I am writing some more bindings, just enough to write a
> > stateless decoder driver. I hope to finish it in time for the media
> > summit. It will give us a more in-depth idea of the pros and cons here.
>
> --
> Regards,
>
> Laurent Pinchart
>

2023-05-01 20:24:22

by Asahi Lina

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On 02/05/2023 05.10, Nicolas Dufresne wrote:
> Le mercredi 26 avril 2023 à 20:25 +0300, Laurent Pinchart a écrit :
>> Hi Daniel,
>>
>> On Wed, Apr 26, 2023 at 06:14:33PM +0100, Daniel Almeida wrote:
>>> Hi,
>>>
>>> As I said higher up on this thread, I can maintain the Rust bits and
>>> help out with the issues around it.
>>>
>>> IMHO, we should at least try this. Who knows, it might work out :)
>>>
>>> Laurent, maybe we can take a piecemeal approach? Right now there are no
>>> bindings for MC, but I wouldn't complain about fixing some of the C code
>>> when the time comes.
>>
>> The lifetime issues affect plain V4L2 video nodes too I'm afraid :-)
>
> Everything under the bindings is unsafe code, so it does not prevent doing upper
> implementation and have other things be memory safe. It just make Rust less
> helpful in some cases (I guess everything across ops).
>
> There is low hanging fruit if some folks are interested. I see legitimate
> benefit in rewriting in rust the JPEG parser, the H.264 reference list
> generator, and maybe VP9 probability update lib. AV1 driver will need a lib to
> reduce duplicates, this could be done straight in Rust (offering a C interface
> of course, so it does not matter if the users are written in rust or C).

Unfortunately I don't think actually replacing the C implementations
will be possible until Rust architecture support is on par with C, which
probably means waiting until gccrs is ready...

We could have both implementations until then (and only use the C one
where Rust doesn't work), but the code duplication has an extra
maintenance cost so it's not free. That's why people are mostly focusing
on drivers first instead of core code.

~~ Lina

2023-05-01 20:26:01

by Nicolas Dufresne

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

Le mardi 02 mai 2023 à 05:17 +0900, Asahi Lina a écrit :
> On 02/05/2023 05.10, Nicolas Dufresne wrote:
> > Le mercredi 26 avril 2023 à 20:25 +0300, Laurent Pinchart a écrit :
> > > Hi Daniel,
> > >
> > > On Wed, Apr 26, 2023 at 06:14:33PM +0100, Daniel Almeida wrote:
> > > > Hi,
> > > >
> > > > As I said higher up on this thread, I can maintain the Rust bits and
> > > > help out with the issues around it.
> > > >
> > > > IMHO, we should at least try this. Who knows, it might work out :)
> > > >
> > > > Laurent, maybe we can take a piecemeal approach? Right now there are no
> > > > bindings for MC, but I wouldn't complain about fixing some of the C code
> > > > when the time comes.
> > >
> > > The lifetime issues affect plain V4L2 video nodes too I'm afraid :-)
> >
> > Everything under the bindings is unsafe code, so it does not prevent doing upper
> > implementation and have other things be memory safe. It just make Rust less
> > helpful in some cases (I guess everything across ops).
> >
> > There is low hanging fruit if some folks are interested. I see legitimate
> > benefit in rewriting in rust the JPEG parser, the H.264 reference list
> > generator, and maybe VP9 probability update lib. AV1 driver will need a lib to
> > reduce duplicates, this could be done straight in Rust (offering a C interface
> > of course, so it does not matter if the users are written in rust or C).
>
> Unfortunately I don't think actually replacing the C implementations
> will be possible until Rust architecture support is on par with C, which
> probably means waiting until gccrs is ready...
>
> We could have both implementations until then (and only use the C one
> where Rust doesn't work), but the code duplication has an extra
> maintenance cost so it's not free. That's why people are mostly focusing
> on drivers first instead of core code.

Didn't know that, let's postpone this idea then.

thanks,
Nicolas

>
> ~~ Lina
>
>

2023-05-02 19:19:55

[permalink] [raw]

Subject: Re: [PATCH 0/6] Initial Rust V4L2 support

On Mon, May 1, 2023 at 10:17 PM Asahi Lina <[email protected]> wrote:
>
> Unfortunately I don't think actually replacing the C implementations
> will be possible until Rust architecture support is on par with C, which
> probably means waiting until gccrs is ready...

There is also a second approach via `rustc_codegen_gcc`: Antoni (Cc'd)
showed in Kangrejos and LPC last year that it could compile the Rust
kernel code (with a few tweaks).

Cheers,
Miguel

2023-05-03 11:05:15