public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH V1 0/2] Rust DRM build fix
@ 2026-04-26  8:22 Mukesh Kumar Chaurasiya (IBM)
  2026-04-26  8:22 ` [PATCH V1 1/2] rust/drm: import ARef from sync crate Mukesh Kumar Chaurasiya (IBM)
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Mukesh Kumar Chaurasiya (IBM) @ 2026-04-26  8:22 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, simona, dakr,
	aliceryhl, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	tmgross, daniel.almeida, lina+kernel, j, mkchauras, dri-devel,
	linux-kernel, rust-for-linux

rust drm gem shmem doesn't builds due to an import error of ARef from
types instead of sync.

This series fixes that and also makes CONFIG_RUST_DRM_GEM_SHMEM_HELPER
dependent of RUST and making it selectable from menuconfig.

Mukesh Kumar Chaurasiya (IBM) (2):
  rust/drm: import ARef from sync crate
  drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig

 drivers/gpu/drm/Kconfig      | 4 ++--
 rust/kernel/drm/gem/shmem.rs | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH V1 1/2] rust/drm: import ARef from sync crate
  2026-04-26  8:22 [PATCH V1 0/2] Rust DRM build fix Mukesh Kumar Chaurasiya (IBM)
@ 2026-04-26  8:22 ` Mukesh Kumar Chaurasiya (IBM)
  2026-04-28  5:21   ` Claude review: " Claude Code Review Bot
  2026-04-26  8:22 ` [PATCH V1 2/2] drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig Mukesh Kumar Chaurasiya (IBM)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Mukesh Kumar Chaurasiya (IBM) @ 2026-04-26  8:22 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, simona, dakr,
	aliceryhl, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	tmgross, daniel.almeida, lina+kernel, j, mkchauras, dri-devel,
	linux-kernel, rust-for-linux

ARef is defined in sync and is getting used from types causing the
build to fail.

Fix this by using ARef from sync crate.

Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
---
 rust/kernel/drm/gem/shmem.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index d025fb035195..50aa57bf78ca 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -19,8 +19,8 @@
     },
     error::to_result,
     prelude::*,
+    sync::aref::ARef,
     types::{
-        ARef,
         Opaque, //
     }, //
 };
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH V1 2/2] drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig
  2026-04-26  8:22 [PATCH V1 0/2] Rust DRM build fix Mukesh Kumar Chaurasiya (IBM)
  2026-04-26  8:22 ` [PATCH V1 1/2] rust/drm: import ARef from sync crate Mukesh Kumar Chaurasiya (IBM)
@ 2026-04-26  8:22 ` Mukesh Kumar Chaurasiya (IBM)
  2026-04-28  5:21   ` Claude review: " Claude Code Review Bot
  2026-04-27  6:21 ` [PATCH V1 0/2] Rust DRM build fix Thomas Zimmermann
  2026-04-28  5:21 ` Claude review: " Claude Code Review Bot
  3 siblings, 1 reply; 7+ messages in thread
From: Mukesh Kumar Chaurasiya (IBM) @ 2026-04-26  8:22 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, simona, dakr,
	aliceryhl, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg,
	tmgross, daniel.almeida, lina+kernel, j, mkchauras, dri-devel,
	linux-kernel, rust-for-linux

CONFIG_RUST_DRM_GEM_SHMEM_HELPER doesn't have a description leading to
not show up as a selectable menu item in menuconfig. Along side it's not
dependent on rust.

Fix this by making CONFIG_RUST_DRM_GEM_SHMEM_HELPER dependent on
CONFIG_RUST and give it a description string so that it can be selected
from menuconfig.

Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
---
 drivers/gpu/drm/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 8f5a8d3012e4..89adb700d710 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -269,8 +269,8 @@ config DRM_GEM_SHMEM_HELPER
 	  Choose this if you need the GEM shmem helper functions
 
 config RUST_DRM_GEM_SHMEM_HELPER
-	bool
-	depends on DRM && MMU
+	bool "Rust GEM shmem helper functions"
+	depends on DRM && MMU && RUST
 	select DRM_GEM_SHMEM_HELPER
 	help
 	  Choose this if you need the GEM shmem helper functions In Rust
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V1 0/2] Rust DRM build fix
  2026-04-26  8:22 [PATCH V1 0/2] Rust DRM build fix Mukesh Kumar Chaurasiya (IBM)
  2026-04-26  8:22 ` [PATCH V1 1/2] rust/drm: import ARef from sync crate Mukesh Kumar Chaurasiya (IBM)
  2026-04-26  8:22 ` [PATCH V1 2/2] drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig Mukesh Kumar Chaurasiya (IBM)
@ 2026-04-27  6:21 ` Thomas Zimmermann
  2026-04-28  5:21 ` Claude review: " Claude Code Review Bot
  3 siblings, 0 replies; 7+ messages in thread
From: Thomas Zimmermann @ 2026-04-27  6:21 UTC (permalink / raw)
  To: Mukesh Kumar Chaurasiya (IBM), maarten.lankhorst, mripard,
	airlied, simona, dakr, aliceryhl, ojeda, boqun, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, daniel.almeida, lina+kernel, j,
	dri-devel, linux-kernel, rust-for-linux

Hi

Am 26.04.26 um 10:22 schrieb Mukesh Kumar Chaurasiya (IBM):
> rust drm gem shmem doesn't builds due to an import error of ARef from
> types instead of sync.
>
> This series fixes that and also makes CONFIG_RUST_DRM_GEM_SHMEM_HELPER
> dependent of RUST and making it selectable from menuconfig.

Memory managers are selected by drivers, not by users. You have to fix 
the driver rules instead.

Best regards
Thomas

>
> Mukesh Kumar Chaurasiya (IBM) (2):
>    rust/drm: import ARef from sync crate
>    drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig
>
>   drivers/gpu/drm/Kconfig      | 4 ++--
>   rust/kernel/drm/gem/shmem.rs | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Claude review: Rust DRM build fix
  2026-04-26  8:22 [PATCH V1 0/2] Rust DRM build fix Mukesh Kumar Chaurasiya (IBM)
                   ` (2 preceding siblings ...)
  2026-04-27  6:21 ` [PATCH V1 0/2] Rust DRM build fix Thomas Zimmermann
@ 2026-04-28  5:21 ` Claude Code Review Bot
  3 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  5:21 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Rust DRM build fix
Author: "Mukesh Kumar Chaurasiya (IBM)" <mkchauras@gmail.com>
Patches: 4
Reviewed: 2026-04-28T15:21:52.284046

---

This is a 2-patch series fixing build issues with the Rust DRM GEM shmem helper code. Patch 1 fixes a genuine import error (`ARef` moved from `types` to `sync::aref`). Patch 2 makes the Kconfig option user-selectable and adds a `RUST` dependency. 

Patch 1 is a correct and necessary fix. Patch 2 has issues — adding the `RUST` dependency is fine, but changing the option from a hidden `bool` to a user-selectable one deviates from the established kernel pattern for helper configs and should be reconsidered.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Claude review: rust/drm: import ARef from sync crate
  2026-04-26  8:22 ` [PATCH V1 1/2] rust/drm: import ARef from sync crate Mukesh Kumar Chaurasiya (IBM)
@ 2026-04-28  5:21   ` Claude Code Review Bot
  0 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  5:21 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Verdict: Correct fix, minor style nit.**

The fix is valid. In the current tree, `shmem.rs` imports `ARef` from `crate::types`, but `ARef` is defined in `rust/kernel/sync/aref.rs` (line 68: `pub struct ARef<T: AlwaysRefCounted>`), and no re-export exists in `types.rs`. This is a real build breakage.

The change:
```rust
-    types::{
-        ARef,
-        Opaque, //
-    }, //
+    sync::aref::ARef,
+    types::{
+        Opaque, //
+    }, //
```

This correctly moves the import. One minor observation: the trailing `//` comments on the closing braces (lines like `}, //`) look like they're artifacts of `rustfmt` line-breaking workarounds from the existing code, not something the patch introduces, so that's fine.

**No blocking issues.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Claude review: drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig
  2026-04-26  8:22 ` [PATCH V1 2/2] drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig Mukesh Kumar Chaurasiya (IBM)
@ 2026-04-28  5:21   ` Claude Code Review Bot
  0 siblings, 0 replies; 7+ messages in thread
From: Claude Code Review Bot @ 2026-04-28  5:21 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Verdict: Adding `RUST` dependency is correct. Making it user-selectable is questionable.**

The change:
```diff
 config RUST_DRM_GEM_SHMEM_HELPER
-	bool
-	depends on DRM && MMU
+	bool "Rust GEM shmem helper functions"
+	depends on DRM && MMU && RUST
```

**Adding `depends on RUST`** — This is a correct fix. Rust code obviously cannot build without `CONFIG_RUST`. Good catch.

**Adding the description string to make it user-selectable** — This is problematic. The C counterpart `DRM_GEM_SHMEM_HELPER` is a hidden `tristate` with no description (line 262 of Kconfig: `tristate` with no prompt string). It follows the standard kernel pattern where helper/library Kconfig options are hidden and get enabled via `select` from the drivers that need them. Users don't manually enable helper code — drivers pull in what they need.

`RUST_DRM_GEM_SHMEM_HELPER` was designed to follow this same pattern: a hidden `bool` that a Rust DRM driver would `select`. Currently nothing selects it (no Rust DRM drivers in-tree yet), so making it user-selectable is presumably to allow testing. But this should be documented in the commit message — the current message doesn't explain *why* a user would want to manually enable a helper library that no driver uses.

If the intent is just testability, a better approach might be to keep it hidden and add a `RUST_DRM_KUNIT_TEST` or similar config that selects it, following the pattern used elsewhere in the kernel for testing infrastructure code.

**Additional concern:** The commit message says "Along side it's not dependent on rust" — this is grammatically unclear and should be "Additionally, it is not dependent on RUST" or similar. Minor, but commit messages are permanent documentation.

**Commit message nit:** The cover letter says "making it selectable from menuconfig" but doesn't explain the use case for a user manually enabling a helper library with no consumers.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-28  5:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-26  8:22 [PATCH V1 0/2] Rust DRM build fix Mukesh Kumar Chaurasiya (IBM)
2026-04-26  8:22 ` [PATCH V1 1/2] rust/drm: import ARef from sync crate Mukesh Kumar Chaurasiya (IBM)
2026-04-28  5:21   ` Claude review: " Claude Code Review Bot
2026-04-26  8:22 ` [PATCH V1 2/2] drm: Make CONFIG_RUST_DRM_GEM_SHMEM_HELPER selectable from menuconfig Mukesh Kumar Chaurasiya (IBM)
2026-04-28  5:21   ` Claude review: " Claude Code Review Bot
2026-04-27  6:21 ` [PATCH V1 0/2] Rust DRM build fix Thomas Zimmermann
2026-04-28  5:21 ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox