gt; to_result(unsafe {
> @@ -154,10 +141,12 @@ fn create_handle(
> }
>
> /// Looks up an object by its handle for a given `File`.
> -fn lookup_handle(
> -file: &drm::File<<::Driver as
> drm::Driver>::File>,
> -handle: u32,
> -) -> Result> {
> +fn lookup_handle(file: &drm::File, handle: u32) ->
> Result>
> +where
> +Self: AllocImpl,
> +D: drm::Driver,
> +F: drm::file::DriverFile,
Same here?
> +{
> // SAFETY: The arguments are all valid per the type invariants.
> let ptr = unsafe {
> bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
> if ptr.is_null() {
> @@ -212,8 +201,8 @@ impl Object {
>
> const OBJECT_FUNCS: bindings::drm_gem_object_funcs =
> bindings::drm_gem_object_funcs {
> free: Some(Self::free_callback),
> -open: Some(open_callback::>),
> -close: Some(close_callback::>),
> +open: Some(open_callback::),
> +close: Some(close_callback::),
> print_info: None,
> export: None,
> pin: None,
> @@ -296,6 +285,8 @@ fn deref(&self) -> &Self::Target {
> }
>
> impl AllocImpl for Object {
> +type Driver = T::Driver;
> +
> const ALLOC_OPS: AllocOps = AllocOps {
> gem_create_object: None,
> prime_handle_to_fd: None,
> --
> 2.50.0
>
With the DriverFile comment sorted out:
Reviewed-by: Daniel Almeida
an object with the given size
> diff --git a/include/drm/drm_gem_shmem_helper.h
> b/include/drm/drm_gem_shmem_helper.h
> index 92f5db84b9c22..235dc33127b9a 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -107,6 +107,7 @@ struct drm_gem_shmem_object {
> #define to_drm_gem_shmem_obj(obj) \
> container_of(obj, struct drm_gem_shmem_object, base)
>
> +int drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object
> *shmem, size_t size);
> struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev,
> size_t size);
> struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device
> *dev,
> size_t size,
> --
> 2.50.0
>
>
Reviewed-by: Daniel Almeida
hmem_object *drm_gem_shmem_create_with_mnt(struct drm_device
> *dev,
> size_t size,
> struct vfsmount *gemfs);
> +void drm_gem_shmem_release(struct drm_gem_shmem_object *shmem);
> void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem);
>
> void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem);
> --
> 2.50.0
>
>
Reviewed-by: Daniel Almeida
on or the shmem layer,
> depending on their needs.
>
> Signed-off-by: Asahi Lina
> Signed-off-by: Daniel Almeida
> Signed-off-by: Lyude Paul
>
> ---
>
> V2:
> * Use the drm_gem_shmem_init() and drm_gem_shmem_release() that I extracted
> so we can handle memory a
om_raw(dma_ptr) };
> +
> +// INVARIANT: We used drm_gem_prime_export() to create this dma_buf,
> fulfilling the
> +// invariant that this dma_buf came from a GEM object of type `Self`.
> +Ok(DmaBuf(dma_buf.into(), PhantomData))
> +}
> +
> /// Creates an mmap offset to map the object from userspace.
> fn create_mmap_offset(&self) -> Result {
> // SAFETY: The arguments are valid per the type invariant.
> --
> 2.50.0
>
Reviewed-by: Daniel Almeida
> On 29 Aug 2025, at 19:35, Lyude Paul wrote:
>
> This introduces an optional export() callback for GEM objects, which is
> used to implement the drm_gem_object_funcs->export function.
>
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/nova/gem.rs | 1 +
> rust/kernel/drm/gem/mod.rs |
> On 29 Aug 2025, at 19:35, Lyude Paul wrote:
>
> For retrieving a pointer to the struct dma_resv for a given GEM object. We
> also introduce it in a new trait, BaseObjectPrivate, which we automatically
> implement for all gem objects and don't expose to users outside of the
> crate.
>
> Sign
Hi Lyude,
> On 29 Aug 2025, at 19:35, Lyude Paul wrote:
>
> Currently we expose the ability to retrieve an SGTable for an shmem gem
> object using gem::shmem::Objectsg_table(). However, this only gives us
> a
> borrowed reference. This being said - retrieving an SGTable is a fallible
> op
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index fcffc3988a903..59242d83efe21 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -81,6 +81,7 @@
> pub mod device_id;
> pub mod devres;
> pub mod dma;
> +pub mod dma_buf;
> pub mod driver;
> #[cfg(CONFIG_DRM = "y")]
> pub mod drm;
> --
> 2.50.0
>
Reviewed-by: Daniel Almeida
> On 29 Aug 2025, at 19:35, Lyude Paul wrote:
>
> This is an associated type that may be used in order to specify a data-type
> to pass to gem objects when construction them, allowing for drivers to more
This doesn’t read very well IMHO.
> easily initialize their private-data for gem objects
> On 29 Aug 2025, at 19:35, Lyude Paul wrote:
>
> One of the original intents with the gem bindings was that drivers could
> specify additional gem implementations, in order to enable for driver
> private gem objects. This wasn't really possible however, as up until now
> our GEM bindings have
@@ pub struct Object {
> }
>
> impl Object {
> -/// The size of this object's structure.
> -pub const SIZE: usize = mem::size_of::();
> -
> const OBJECT_FUNCS: bindings::drm_gem_object_funcs =
> bindings::drm_gem_object_funcs {
> free: Some(Self::free_callback),
> open: Some(open_callback::),
> --
> 2.50.0
>
>
Reviewed-by: Daniel Almeida
-95,7 +96,7 @@ extern "C" fn close_callback(
> raw_file: *mut bindings::drm_file,
> ) {
> // SAFETY: `open_callback` is only ever called with a valid pointer to a
> `struct drm_file`.
> -let file = unsafe { drm::File::< drm::Driver>::File>::from_raw(raw_file) };
> +let file = unsafe { DriverFilefrom_raw(raw_file) };
>
> // SAFETY: `close_callback` is specified in the AllocOps structure for
> `Object`, ensuring
> // that `raw_obj` is indeed contained within a `Object`.
> --
> 2.50.0
>
>
Reviewed-by: Daniel Almeida
> On 4 Sep 2025, at 00:16, Alexandre Courbot wrote:
>
> On Thu Sep 4, 2025 at 12:15 AM JST, Joel Fernandes wrote:
>
+use kernel::prelude::*;
+
+/// Macro for defining bitfield-packed structures in Rust.
+/// The size of the underlying storage type is specified with
#
> On 3 Sep 2025, at 18:54, Joel Fernandes wrote:
>
> Out of broad need for these macros in Rust, move them out. Several folks
> have shown interest (Nova, Tyr GPU drivers).
>
> bitstruct - defines bitfields in Rust structs similar to C.
> register - support for defining hardware registers and
Hi Joel,
> On 24 Aug 2025, at 10:59, Joel Fernandes wrote:
>
> Add a minimal bitfield library for defining in Rust structures (called
> bitstruct), similar in concept to bit fields in C structs. This will be used
> for defining page table entries and other structures in nova-core.
>
> Signed-of
er
> trees respectively. Until then, drm-rust provides a dedicated place to
> coordinate development without disrupting existing workflows too much.
>
> Cc: Alice Ryhl
> Cc: David Airlie
> Cc: Simona Vetter
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas
Hi Lyude, thanks a lot for working on this! :)
> On 29 Aug 2025, at 19:35, Lyude Paul wrote:
>
> Now that my rust skills have been honed, I noticed that there's a lot of
> generics in our gem bindings that don't actually need to be here. Currently
> the hierarchy of traits in our gem bindings lo
Add an initial test suit covering query device properties, allocating
memory, binding and unbinding VA ranges through VM_BIND and submitting a
simple piece of work through GROUP_SUBMIT.
---
lib/igt_panthor.c | 136 ++
lib/igt_panthor.h | 20 +++
tests/panth
Add the necessary code needed to compile panthor tests. The tests
themselves will be added in a subsequent patch.
Signed-off-by: Daniel Almeida
---
meson.build | 8
tests/meson.build | 2 ++
tests/panthor/meson.build | 11 +++
3 files changed, 21
Add the basic infrastructure that will be used by the Panthor tests
themselves.
Signed-off-by: Daniel Almeida
---
lib/igt_panthor.c | 14 ++
lib/igt_panthor.h | 8
lib/meson.build | 1 +
3 files changed, 23 insertions(+)
create mode 100644 lib/igt_panthor.c
create
We will be adding tests for Panthor in a subsequent patch, so first add
the ability to open the device.
Signed-off-by: Daniel Almeida
---
lib/drmtest.c | 1 +
lib/drmtest.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 436b6de78..f4b429048 100644
check this submission with as far as I am aware.
Daniel Almeida (4):
lib: add support for opening Panthor devices
tests: panthor: add initial infrastructure
lib: initial panthor infrastructure
tests/panthor: add panthor tests
lib/drmtest.c | 1 +
lib/drmtest.h |
ures and
places.
Co-developed-by: Alice Ryhl
Signed-off-by: Alice Ryhl
Co-developed-by: Beata Michalska
Signed-off-by: Beata Michalska
Co-developed-by: Carsten Haitzler
Signed-off-by: Carsten Haitzler
Co-developed-by: Rob Herring
Signed-off-by: Rob Herring
Signed-off-by: Daniel Almeida
--
Hi Benno,
> On 2 Aug 2025, at 17:58, Benno Lossin wrote:
>
> On Sat Aug 2, 2025 at 4:15 PM CEST, Daniel Almeida wrote:
>> On 2 Aug 2025, at 07:42, Benno Lossin wrote:
>>> On Fri Aug 1, 2025 at 11:22 PM CEST, Daniel Almeida wrote:
>>>> One thing I didn’t
Hi Onur,
> On 5 Aug 2025, at 06:08, Onur Özkan wrote:
>
> On Sat, 2 Aug 2025 11:15:07 -0300
> Daniel Almeida wrote:
>
>> Btw, I can also try to implement a proof of concept, so long as
>> people agree that this approach makes sense.
>
> It's not necessa
> On 2 Aug 2025, at 07:42, Benno Lossin wrote:
>
> On Fri Aug 1, 2025 at 11:22 PM CEST, Daniel Almeida wrote:
>> Hi Benno,
>>
>>> On 7 Jul 2025, at 16:48, Benno Lossin wrote:
>>>
>>> On Mon Jul 7, 2025 at 8:06 PM CEST, Onur wrote:
>>&g
/gem/mod.rs | 2 +-
> rust/kernel/drm/ioctl.rs | 2 +-
> 7 files changed, 12 insertions(+), 6 deletions(-)
>
> —
> 2.50.1
Reviewed-by: Daniel Almeida
Patch 3 alone indeed produces the following warnings:
warning: srctree/ link to include/linux/blk_mq.h does not
[…]
>>>
The idea of register blocks is interesting. I wonder how that would
translate in terms of access to invididual registers, i.e. does the
block end up just being a prefix into the full register name, or is it
something else? From your example declaration I picture that a
[…]
>
> Assuming that the @io_fixed stuff is correct:
>
> Reviewed-by: Daniel Almeida
>
> — Daniel
For the stuff in macros.rs <http://macros.rs/>.
I did not check the device-specific part in Nova, as I’m not familiar with
how it works.
— Daniel
{
> +if idx < Self::SIZE {
> + Ok(self.write(io, idx))
> +} else {
> +Err(EINVAL)
> +}
> +}
> +
> +/// Read the array register at index `idx` in `io` and run `f`
> on its value to obtain a
> +/// new value to write back.
> +///
> +/// The validity of `idx` is checked at run-time, and `EINVAL`
> is returned is the
> +/// access was out-of-bounds.
> +#[inline(always)]
> +pub(crate) fn try_alter(
> +io: &T,
> +idx: usize,
> +f: F,
> +) -> ::kernel::error::Result where
> +T: ::core::ops::Deref>,
> +F: ::core::ops::FnOnce(Self) -> Self,
> +{
> +if idx < Self::SIZE {
> +Ok(Self::alter(io, idx, f))
> +} else {
> +Err(EINVAL)
> +}
> +}
> +}
> +};
> }
>
> --
> 2.50.1
>
Assuming that the @io_fixed stuff is correct:
Reviewed-by: Daniel Almeida
— Daniel
Hi Alex,
> On 18 Jul 2025, at 04:26, Alexandre Courbot wrote:
>
> The relative registers are currently very unsafe to use: callers can
> specify any constant as the base address for access, meaning they can
> effectively interpret any I/O address as any relative register.
>
> Ideally, valid bas
@@ pub(crate) fn read(
> Self(io.read32(base + $offset))
> }
>
> -#[inline]
> +#[inline(always)]
> pub(crate) fn write(
> self,
> io: &T,
> @@ -406,7 +406,7 @@ pub(crate) fn write(
> io.write32(self.0, base + $offset)
> }
>
> -#[inline]
> +#[inline(always)]
> pub(crate) fn alter(
> io: &T,
> base: usize,
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
ame:ident @ + $offset:literal) => {
> +(@io_relative $name:ident @ + $offset:literal) => {
> #[allow(dead_code)]
> impl $name {
> pub(crate) const OFFSET: usize = $offset;
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
#[allow(unused_mut)]
> +let mut value = Self(Default::default());
> +
> +::kernel::macros::paste!(
> +$(
> +value.[](Default::default());
> +)*
> +);
> +
> +value
> + }
> +}
> +};
> +
> // Generates the IO accessors for a fixed offset register.
> (@io $name:ident @ $offset:expr) => {
> #[allow(dead_code)]
>
> --
> 2.50.1
>
>
Also very neat.
Reviewed-by: Daniel Almeida
f.debug_struct(stringify!($name))
> +.field("", &format_args!("{:#x}", &self.0))
> +$(
> +.field(stringify!($field), &self.$field())
> +)*
> +.finish()
> +}
> +}
> +};
> +
> // Generates the IO accessors for a fixed offset register.
> (@io $name:ident @ $offset:expr) => {
> #[allow(dead_code)]
>
> --
> 2.50.1
>
Reviewed-by: Daniel Almeida
t; +register!(@field_accessors $name {
> + $(
> +$hi:$lo $field as $type
> +$(?=> $try_into_type)?
> +$(=> $into_type)?
> +$(, $comment)?
> +;
> +)*
> +});
> };
>
> // Defines all the field getter/methods methods for `$name`.
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
io.write32(self.0, $offset)
> }
>
> +/// Read the register from its address in `io` and run `f` on
> its value to obtain a new
> +/// value to write back.
Ah, really neat!
> #[inline]
> pub(crate) fn alter(
> io: &T,
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
value: $to_type)
> -> Self {
> );
> };
>
> -// Creates the IO accessors for a fixed offset register.
> +// Generates the IO accessors for a fixed offset register.
> (@io $name:ident @ $offset:expr) => {
> #[allow(dead_code)]
> i
space needed to store the actual constant in the
binary.
Again, not sure whether this is feasible.
> #[inline]
> pub(crate) fn read(io: &T) -> Self where
> T: ::core::ops::Deref>,
> @@ -351,6 +348,8 @@ pub(crate) fn alter(
> (@io $name:ident @ + $offset:literal) => {
> #[allow(dead_code)]
> impl $name {
> +pub(crate) const OFFSET: usize = $offset;
> +
> #[inline]
> pub(crate) fn read(
> io: &T,
>
> --
> 2.50.1
>
Reviewed-by: Daniel Almeida
t; -pub(crate) fn try_alter(
> -io: &T,
> -base: usize,
> -f: F,
> -) -> ::kernel::error::Result<()> where
> -T: ::core::ops::Deref>,
> - F: ::core::ops::FnOnce(Self) -> Self,
> -{
> -let reg = f(Self::try_read(io, base)?);
> -reg.try_write(io, base)
> -}
> }
> };
> }
>
> --
> 2.50.1
>
Reviewed-by: Daniel Almeida
a single field.
> (
> -@leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:ty
> +@leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
> { $process:expr } $to_type:ty => $res_type:ty $(,
> $comment:literal)?;
> ) => {
> ::kernel::macros::paste!(
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
US => SCRATCH, "Boot status of the firmware" {
> /// 0:0 completed as bool, "Whether the firmware has completed
> booting";
> +/// });
> /// ```
> ///
> -/// In this example, `SCRATCH_0_BOOT_STATUS` uses the same I/O address as
> `SCRATCH_0`, while also
> -/// providing its own `completed` method.
> +/// In this example, `SCRATCH_0_BOOT_STATUS` uses the same I/O address as
> `SCRATCH`, while also
> +/// providing its own `completed` field.
> macro_rules! register {
> // Creates a register at a fixed offset of the MMIO space.
> (
>
> --
> 2.50.1
>
Reviewed-by: Daniel Almeida
$(,
> $comment:literal)?;
> ) => {
> ::kernel::macros::paste!(
> -const [<$field:upper>]: ::core::ops::RangeInclusive = $lo..=$hi;
> +const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive =
> $lo..=$hi;
> const [<$field:upper _MASK>]: u32 = 1 << $hi) - 1) << 1) + 1) -
> ((1 << $lo) - 1);
> const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper
> _MASK>].trailing_zeros();
> );
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
};
>
> // Creates a alias register of relative offset register `alias` with its
> own fields.
>
> --
> 2.50.1
>
>
Reviewed-by: Daniel Almeida
This defines a `BOOT_0` type which can be read or written from offset
> `0x100` of an `Io`
> -/// region. It is composed of 3 fields, for instance `minor_revision` is
> made of the 4 less
> +/// region. It is composed of 3 fields, for instance `minor_revision` is
> made of the 4 least
> /// significant bits of the register. Each field can be accessed and modified
> using accessor
> /// methods:
> ///
>
> --
> 2.50.1
>
Reviewed-by: Daniel Almeida
> On 24 Jul 2025, at 19:27, Danilo Krummrich wrote:
>
> On Thu Jul 24, 2025 at 11:06 PM CEST, Lyude Paul wrote:
>> On Thu, 2025-07-24 at 22:03 +0200, Danilo Krummrich wrote:
>>> On Thu Jul 24, 2025 at 9:15 PM CEST, Lyude Paul wrote:
-// SAFETY: All gem objects are refcounted.
-unsafe
t;
> Signed-off-by: Beata Michalska
> Acked-by: Danilo Krummrich
> Reviewed-by: Boqun Feng
Reviewed-by: Daniel Almeida
> On 21 Jul 2025, at 12:33, Danilo Krummrich wrote:
>
> On Thu Jun 26, 2025 at 6:23 PM CEST, Beata Michalska wrote:
>> With the Opaque, the expectations are that Rust should not
>> make any assumptions on the layout or invariants of the wrapped
>> C types. That runs rather counter to ioctl arg
Hi Danilo,
>
> Yeah, I can pick it up, but it won't be considered for the upcoming merge
> window
> anymore, but for the next. After -rc6 drm-misc is in feature freeze and I also
> already send the PR for Nova.
>
> Daniel, Beata: Is there a reason you need this earlier?
IIUC, this will be merg
Hi Chia-I Wu :)
> On 19 Jul 2025, at 21:01, Chia-I Wu wrote:
>
> This series adds devcoredump support to panthor.
>
> This is written from scratch and is not based on the prior work[1]. The
> main differences are
I wonder why this was started from scratch? IIRC, that work stopped, among
other
Hi Miguel,
>> Hmm, I must say I did not know that this was a thing.
>>
>> Why is it better than [#allow] during the development phase?
>
> I have some notes at:
>
>https://docs.kernel.org/rust/coding-guidelines.html#lints
>
> Generally speaking, we default to `expect` unless there is a rea
Hi Steven,
> On 30 Jun 2025, at 07:11, Steven Price wrote:
>
> Hi Daniel,
>
> My Rust is still quite weak, so I'll just review the GPU-specific parts.
> Please CC me on future posts.
I just realized I forgot about cc’ing the current Panthor maintainers. My bad.
>> +
>> +fn issue_soft_reset(io
Hi Rob,
> On 30 Jun 2025, at 10:52, Rob Herring wrote:
>
> On Sat, Jun 28, 2025 at 4:31 AM Miguel Ojeda
> wrote:
>>
>> On Sat, Jun 28, 2025 at 2:13 AM Daniel Almeida
>> wrote:
>>>
>>> Also, for some reason the Clippy lint did not save me
Hi Maíra, thanks for chiming in :)
>
> To enhance readability, consider using a regmap similar to
> panthor_regs.h. This would help avoid 'magic numbers' and make the
> code's intent much clearer.
Are you referring to "struct regmap" itself? Because last I checked, this
abstraction is not avail
Hi Miguel,
> On 28 Jun 2025, at 06:44, Miguel Ojeda
> wrote:
>
> Hi Daniel,
>
> Some procedural notes and general comments, and please note that some
> may apply several times.
>
> On Sat, Jun 28, 2025 at 12:35 AM Daniel Almeida
> wrote:
>>
>> Sig
Hi Danilo, thank you an Boqun for having a look at this,
> On 27 Jun 2025, at 20:12, Danilo Krummrich wrote:
>
> On Fri, Jun 27, 2025 at 07:34:04PM -0300, Daniel Almeida wrote:
>> +#[pin_data]
>> +pub(crate) struct TyrData {
>> +pub(crate) pdev: ARef,
>&
I’ll fix the missing “rust: drm:” tags on a v2.
— Daniel
Signed-off-by: Alice Ryhl
Co-developed-by: Beata Michalska
Signed-off-by: Beata Michalska
Co-developed-by: Carsten Haitzler
Signed-off-by: Carsten Haitzler
Co-developed-by: Rob Herring
Signed-off-by: Rob Herring
Signed-off-by: Daniel Almeida
---
The development of Tyr itself started in
Hmm, this has an issue
[..]
>
> +impl, T> Drop for MmInner {
> +fn drop(&mut self) {
> +// SAFETY: If the MmInner is dropped then all nodes are gone (since
> they hold references),
> +// so it is safe to tear down the allocator.
> +unsafe {
> +bindings::d
Hi Miguel
> On 25 Jun 2025, at 09:47, Miguel Ojeda
> wrote:
>
> On Tue, Jun 24, 2025 at 12:13 AM Daniel Almeida
> wrote:
>>
>> Signed-off-by: Asahi Lina
>
> Patches from others also need to be signed off by you as carrier.
Thanks for catching that. This w
0d6b..f369da5b12fb876f23eda8ea7665990919f3960c
100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -7,6 +7,7 @@
pub mod file;
pub mod gem;
pub mod ioctl;
+pub mod mm;
pub use self::device::Device;
pub use self::driver::Driver;
---
base-commit: dc35ddcf97e99b18559d0855071030e664aae44d
change-id: 20250623-topics-tyr-drm_mm-d1d43d436311
Best regards,
--
Daniel Almeida
eemed both somewhat sufficient
and required for upcoming drivers.
Note that all operations provided were tested on a real driver and on a
real device. Specifically, all operations so far are required.
Signed-off-by: Asahi Lina
Co-developed-by: Daniel Almeida
Signed-off-by: Daniel Almeida
---
Hi
Hi Beata,
> There is no concurrent access nor shared references, unless the
> handler decides otherwise
It can’t do so in safe code. There is no way to manufacture a shared
reference from a mutable one in safe code and if it passes that to C, then
it’s already using a unsafe block for the ffi ca
Hi Boris,
> On 19 Jun 2025, at 08:57, Boris Brezillon
> wrote:
>
> Hi,
>
> On Fri, 13 Jun 2025 13:42:59 -0300
> Daniel Almeida wrote:
>
>> Danilo,
>>
>>
>>>
>>>
>>>>>> +// SAFETY: DRM GpuVmBo objects are alw
Hi Benno,
> On 19 Jun 2025, at 08:01, Benno Lossin wrote:
>
> On Thu Jun 19, 2025 at 12:55 PM CEST, Danilo Krummrich wrote:
>> On Thu, Jun 19, 2025 at 12:21:02PM +0200, Beata Michalska wrote:
>>> diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
>>> index 445639404fb7..12b29613167
> On 19 Jun 2025, at 09:26, Daniel Almeida wrote:
>
> Hi Benno,
>
>> On 19 Jun 2025, at 08:01, Benno Lossin wrote:
>>
>> On Thu Jun 19, 2025 at 12:55 PM CEST, Danilo Krummrich wrote:
>>> On Thu, Jun 19, 2025 at 12:21:02PM +0200, Beata Michalska w
Hi Beata,
> On 19 Jun 2025, at 07:21, Beata Michalska wrote:
>
> With the Opaque, the expectations are that Rust should not make any
> assumptions on the layout or invariants of the wrapped C types.
> That runs rather counter to ioctl arguments, which must adhere to
> certain data-layout constra
Danilo,
>
>
+// SAFETY: DRM GpuVmBo objects are always reference counted and the
get/put functions
+// satisfy the requirements.
+unsafe impl AlwaysRefCounted for GpuVmBo {
+fn inc_ref(&self) {
+// SAFETY: The drm_gpuvm_get function satisfies the requi
Hi Danilo,
[…]
>
>> +
>> +impl OpMap {
>> +/// Returns the base address of the new mapping.
>> +#[inline]
>> +pub fn addr(&self) -> u64 {
>> +self.0.va.addr
>> +}
>> +
>> +/// Returns the range of the new mapping.
>> +#[inline]
>> +pub fn range(&self) -> u64 {
Hi Danilo,
> The lock might be held already by the driver or by TTM when things are called
> from TTM callbacks.
>
> This is why GPUVM never takes locks by itself, but asserts that the correct
> lock
> is held.
>
> I think we really want to get proof by the driver by providing lock guard
> refe
Hi Christian
> On 22 May 2025, at 05:44, Christian König wrote:
>
> On 5/21/25 22:29, Lyude Paul wrote:
>> From: Asahi Lina
>>
>> This is just for basic usage in the DRM shmem abstractions for implied
>> locking, not intended as a full DMA Reservation abstraction yet.
>
> Looks good in genera
>
> The lock might be held already by the driver or by TTM when things are called
> from TTM callbacks.
>
> This is why GPUVM never takes locks by itself, but asserts that the correct
> lock
> is held.
>
> I think we really want to get proof by the driver by providing lock guard
> references.
Hi Lyude,
> On 5 Mar 2025, at 19:59, Lyude Paul wrote:
>
> Next up is filling out some of the basic connector hotplugging callbacks -
> which we'll need for setting up the fbdev helpers for KMS devices. Note
> that connector hotplugging in DRM follows a BFL scheme: pretty much all
> probing is p
Hi Lyude,
This patch is similar to the last one, I see that most of my
comments also apply here.
> On 5 Mar 2025, at 19:59, Lyude Paul wrote:
>
> The next step is adding a set of basic bindings to create a plane, which
> has to happen before we can create a CRTC (since we need to be able to at
> On 5 Mar 2025, at 19:59, Lyude Paul wrote:
>
> We start off by introducing wrappers for the first important type of mode
> object: a DRM display connector. This introduces Connector DriverConnector> and ConnectorState. Both
> DriverConnector and DriverConnectorState must be implemented by KM
Hi Lyude,
> On 5 Mar 2025, at 19:59, Lyude Paul wrote:
>
> This commit adds some traits for registering DRM devices with KMS support,
> implemented through the kernel::drm::kms::KmsDriver trait. Devices which
> don't have KMS support can simply use PhantomData.
>
> Signed-off-by: Lyude Paul
>
Hi Lyude,
> On 5 Mar 2025, at 19:59, Lyude Paul wrote:
>
> This adds some very basic rust bindings for fourcc. We only have a single
> format code added for the moment, but this is enough to get a driver
> registered.
>
> Signed-off-by: Lyude Paul
>
> ---
> V3:
> * Drop FormatList and Modifie
ments are valid per the type invariant.
> Ok(unsafe {
> -bindings::drm_vma_node_offset_addr(ptr::addr_of_mut!(
> -(*self.into_gem_obj().get()).vma_node
> -))
> +
> bindings::drm_vma_node_offset_addr(ptr::addr_of_mut!((*self.as_gem_obj()).vma_node))
Hmm, I thought we were on a quest to remove addr_of_mut!() in favor of &raw mut?
Anyways, this is again orthogonal to your patch.
> })
> }
> }
> --
> 2.48.1
>
>
This looks good.
Reviewed-by: Daniel Almeida
get(self.as_raw()) };
> -}
> -
> -unsafe fn dec_ref(obj: NonNull) {
> -// SAFETY: `obj` is a valid pointer to an `Object`.
> -let obj = unsafe { obj.as_ref() };
> -
> -// SAFETY: The safety requirements guarantee that the refcount is
> non-zero.
> -unsafe { bindings::drm_gem_object_put(obj.as_raw()) }
> -}
> -}
> -
> impl super::private::Sealed for Object {}
>
> impl Deref for Object {
> --
> 2.48.1
>
>
Reviewed-by: Daniel Almeida
er is null befoe calling as_ref(),
> ensuring that `ptr` is a
> +// valid pointer to an initialized `Self`.
> +// XXX: The expect lint here is to workaround
> +// https://github.com/rust-lang/rust-clippy/issues/13024
> +#[expect(clippy::undocumented_unsafe_blocks)]
> +let obj = (!ptr.is_null())
> +.then(|| unsafe { Self::as_ref(ptr) })
> +.ok_or(ENOENT)?;
> +
> +// SAFETY:
> +// - We take ownership of the reference of `drm_gem_object_lookup()`.
> +// - Our `NonNull` comes from an immutable reference, thus ensuring
> it is a valid pointer to
> +// `Self`.
> +Ok(unsafe { ARef::from_raw(obj.into()) })
> }
>
> /// Creates an mmap offset to map the object from userspace.
> --
> 2.48.1
>
>
With the extra safety requirement,
Reviewed-by: Daniel Almeida
will live as long as
> // the GEM object lives, hence the pointer must be valid.
> -unsafe { &*self.dev }
> +unsafe { self.dev.as_ref() }
> }
>
> fn as_raw(&self) -> *mut bindings::drm_gem_object {
> --
> 2.48.1
>
>
Reviewed-by: Daniel Almeida
Hi Danilo,
FYI, most of this patch still retains the original code from the Asahi project,
> On 22 Apr 2025, at 18:16, Danilo Krummrich wrote:
>
> (Not a full review, but some things that took my attention from a brief look.)
>
> On Tue, Apr 22, 2025 at 01:41:53PM -0300, Danie
From: Asahi Lina
This is just for basic usage in the DRM shmem abstractions for implied
locking, not intended as a full DMA Reservation abstraction yet.
Signed-off-by: Asahi Lina
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers/dma-resv.c | 13 +
rust/helpers/helper
rs.c | 2 +
rust/kernel/drm/gpuvm.rs| 807
rust/kernel/drm/mod.rs | 2 +
6 files changed, 855 insertions(+)
---
base-commit: e7de41cc4b01dd5500a0c2533c64bdb3f5360567
change-id: 20250320-gpuvm-29d3e0726f34
Best regards,
--
Daniel Almeida
GpuVm::lock(), which is a similar approach as the one
taken by CondVar.
It is up to drivers to make sure that the guard indeed provides the
required locking. Any operations that modifies the interval tree is only
available after the VM has been locked as per above.
Signed-off-by: Asahi Lina
Co-dev
-8bb647b66b1c
Best regards,
--
Daniel Almeida
Hi Miguel, thanks for having a look at this:
> On 24 Mar 2025, at 14:36, Miguel Ojeda
> wrote:
>
> Hi Daniel,
>
> A few quick notes for future versions on style/docs to try to keep
> things consistent upstream -- not an actual review.
>
> On Mon, Mar 24, 202
GpuVm::lock(), which is a similar approach as the one
taken by CondVar.
It is up to drivers to make sure that the guard indeed provides the
required locking. Any operations that modifies the interval tree is only
available after the VM has been locked as per above.
Signed-off-by: Asahi Lina
Co-dev
From: Asahi Lina
This is just for basic usage in the DRM shmem abstractions for implied
locking, not intended as a full DMA Reservation abstraction yet.
Signed-off-by: Asahi Lina
---
rust/bindings/bindings_helper.h | 4 +++-
rust/helpers/dma-resv.c | 13 +
rust/helpers/hel
01dd5500a0c2533c64bdb3f5360567
change-id: 20250320-gpuvm-29d3e0726f34
Best regards,
--
Daniel Almeida
Hi Alyssa,
>
+/**
+ * @DRM_ASAHI_BIND_SINGLE_PAGE: Map a single page of the BO
repeatedly
+ * across the VA range.
+ *
+ * This is useful to fill a VA range with scratch pages or zero
pages.
+ * It is intended as a mechanism t
From: Asahi Lina
Allow a GEM object to share another object's DMA reservation, for use
with drm_gpuvm. To keep memory safety, we hold a reference to the GEM
object owning the resv, and drop it when the child object is freed.
Signed-off-by: Asahi Lina
Signed-off-by: Daniel Almeida
---
From: Asahi Lina
This is just for basic usage in the DRM shmem abstractions for implied
locking, not intended as a full DMA Reservation abstraction yet.
Signed-off-by: Asahi Lina
Signed-off-by: Daniel Almeida
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers/dma-resv.c | 13
From: Asahi Lina
There doesn't seem to be a way for the Rust bindings to get a
compile-time constant reference to drm_gem_shmem_vm_ops, so we need to
duplicate that structure in Rust... this isn't nice...
Signed-off-by: Asahi Lina
Signed-off-by: Daniel Almeida
---
drive
Asahi Lina
Signed-off-by: Daniel Almeida
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index
5ab351409312b5a0de542df2b63627
From: Asahi Lina
This allows drivers to control whether a given GEM object is allowed to
be exported via PRIME to other drivers.
Signed-off-by: Asahi Lina
Signed-off-by: Daniel Almeida
---
rust/kernel/drm/gem/mod.rs | 13 +
rust/kernel/drm/gem/shmem.rs | 4
2 files
drm_gem_private_object_init.
Signed-off-by: Asahi Lina
Signed-off-by: Daniel Almeida
---
drivers/gpu/drm/drm_gem.c | 1 +
drivers/gpu/drm/drm_prime.c | 5 +
include/drm/drm_gem.h | 8
3 files changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
: Daniel Almeida
---
rust/bindings/bindings_helper.h | 3 +
rust/helpers/drm.c | 46 +
rust/helpers/helpers.c | 1 +
rust/helpers/scatterlist.c | 13 ++
rust/kernel/drm/gem/mod.rs | 2 +
rust/kernel/drm/gem/shmem.rs| 422
> On 19 Feb 2025, at 17:23, Dave Airlie wrote:
>
> On Thu, 20 Feb 2025 at 06:22, John Hubbard wrote:
>>
>> On 2/19/25 4:51 AM, Alexandre Courbot wrote:
>>> Yes, that looks like the optimal way to do this actually. It also
>>> doesn't introduce any overhead as the destructuring was doing both
1 - 100 of 159 matches
Mail list logo
