From: Alice Ryhl <aliceryhl@google.com>
To: Laura Nao <laura.nao@collabora.com>
Cc: dakr@kernel.org, airlied@gmail.com, simona@ffwll.ch,
ojeda@kernel.org, boqun@kernel.org, gary@garyguo.net,
bjorn3_gh@protonmail.com, lossin@kernel.org,
a.hindborg@kernel.org, tmgross@umich.edu,
boris.brezillon@collabora.com, dri-devel@lists.freedesktop.org,
nouveau@lists.freedesktop.org, linux-kernel@vger.kernel.org,
rust-for-linux@vger.kernel.org, kernel@collabora.com,
Daniel Almeida <daniel.almeida@collabora.com>
Subject: Re: [PATCH v2 1/1] rust: drm: add RENDER_CAPABILITY flag for render node support
Date: Tue, 5 May 2026 10:54:05 +0000 [thread overview]
Message-ID: <afnMTfVfGq_iLXov@google.com> (raw)
In-Reply-To: <20260505092304.108262-2-laura.nao@collabora.com>
On Tue, May 05, 2026 at 11:23:04AM +0200, Laura Nao wrote:
> Add RENDER_CAPABILITY bool constant to the Driver trait to control
> render node support. When enabled, the driver exposes /dev/dri/renderDXX
> render nodes to userspace. The flag defaults to false, drivers can opt
> in by setting it to true in their Driver implementation.
>
> This is then enabled in the Tyr driver, while it's left disabled for
> Nova for the time being.
>
> Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Laura Nao <laura.nao@collabora.com>
Overall looks good to me!
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> drivers/gpu/drm/tyr/driver.rs | 1 +
> rust/kernel/drm/device.rs | 12 +++++++++++-
> rust/kernel/drm/driver.rs | 5 +++++
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index e20a5978eed6..b7ae37ce3a1b 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -186,6 +186,7 @@ impl drm::Driver for TyrDrmDriver {
> type Object = drm::gem::shmem::Object<BoData>;
>
> const INFO: drm::DriverInfo = INFO;
> + const RENDER_CAPABILITY: bool = true;
>
> kernel::declare_drm_ioctls! {
> (PANTHOR_DEV_QUERY, drm_panthor_dev_query, ioctl::RENDER_ALLOW, TyrDrmFileData::dev_query),
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index adbafe8db54d..e121303d88f0 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -80,6 +80,16 @@ pub struct Device<T: drm::Driver> {
> }
>
> impl<T: drm::Driver> Device<T> {
> + const fn compute_features() -> u32 {
> + let mut features = drm::driver::FEAT_GEM;
> +
> + if T::RENDER_CAPABILITY {
> + features |= drm::driver::FEAT_RENDER;
> + }
> +
> + features
> + }
> +
> const VTABLE: bindings::drm_driver = drm_legacy_fields! {
> load: None,
> open: Some(drm::File::<T::File>::open_callback),
> @@ -105,7 +115,7 @@ impl<T: drm::Driver> Device<T> {
> name: crate::str::as_char_ptr_in_const_context(T::INFO.name).cast_mut(),
> desc: crate::str::as_char_ptr_in_const_context(T::INFO.desc).cast_mut(),
>
> - driver_features: drm::driver::FEAT_GEM,
> + driver_features: Self::compute_features(),
> ioctls: T::IOCTLS.as_ptr(),
> num_ioctls: T::IOCTLS.len() as i32,
> fops: &Self::GEM_FOPS,
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index 5233bdebc9fc..92cbc26ce11f 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
> @@ -16,6 +16,8 @@
>
> /// Driver use the GEM memory manager. This should be set for all modern drivers.
> pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
> +/// Driver supports render nodes, i.e.: /dev/dri/renderDXX devices.
> +pub(crate) const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
>
> /// Information data for a DRM Driver.
> pub struct DriverInfo {
> @@ -115,6 +117,9 @@ pub trait Driver {
>
> /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`.
> const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor];
> +
> + /// Sets the `DRIVER_RENDER` feature for this driver.
> + const RENDER_CAPABILITY: bool = false;
I think it would be nice for this documentation to elaborate more on
what this feature actually does. After all, it clearly took us a while
to understand it, so probably others are confused too.
Something along these lines:
/// Sets the `DRIVER_RENDER` feature for this driver.
///
/// When enabled, the driver exposes `/dev/dri/renderDXX` render nodes to
/// userspace. The render node is an alternate low-priviledge way to access
/// the driver, which is enforced on a per-ioctl level. Userspace processes
/// that open the render node can only invoke ioctls explicitly listed as
/// usable from the render node, whereas userspace processes using the
/// master node can invoke any ioctl.
const RENDER_CAPABILITY: bool = false;
Also, I'd probably call this const `FEAT_RENDER` for consistency / to
make it show up in grep.
Alice
WARNING: multiple messages have this Message-ID (diff)
From: Alice Ryhl <aliceryhl@google.com>
To: Laura Nao <laura.nao@collabora.com>
Cc: dakr@kernel.org, simona@ffwll.ch, ojeda@kernel.org,
boqun@kernel.org, gary@garyguo.net, bjorn3_gh@protonmail.com,
lossin@kernel.org, a.hindborg@kernel.org, tmgross@umich.edu,
boris.brezillon@collabora.com, dri-devel@lists.freedesktop.org,
nouveau@lists.freedesktop.org, linux-kernel@vger.kernel.org,
rust-for-linux@vger.kernel.org, kernel@collabora.com,
Daniel Almeida <daniel.almeida@collabora.com>
Subject: Re: [PATCH v2 1/1] rust: drm: add RENDER_CAPABILITY flag for render node support
Date: Tue, 5 May 2026 10:54:05 +0000 [thread overview]
Message-ID: <afnMTfVfGq_iLXov@google.com> (raw)
In-Reply-To: <20260505092304.108262-2-laura.nao@collabora.com>
On Tue, May 05, 2026 at 11:23:04AM +0200, Laura Nao wrote:
> Add RENDER_CAPABILITY bool constant to the Driver trait to control
> render node support. When enabled, the driver exposes /dev/dri/renderDXX
> render nodes to userspace. The flag defaults to false, drivers can opt
> in by setting it to true in their Driver implementation.
>
> This is then enabled in the Tyr driver, while it's left disabled for
> Nova for the time being.
>
> Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Laura Nao <laura.nao@collabora.com>
Overall looks good to me!
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> drivers/gpu/drm/tyr/driver.rs | 1 +
> rust/kernel/drm/device.rs | 12 +++++++++++-
> rust/kernel/drm/driver.rs | 5 +++++
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index e20a5978eed6..b7ae37ce3a1b 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -186,6 +186,7 @@ impl drm::Driver for TyrDrmDriver {
> type Object = drm::gem::shmem::Object<BoData>;
>
> const INFO: drm::DriverInfo = INFO;
> + const RENDER_CAPABILITY: bool = true;
>
> kernel::declare_drm_ioctls! {
> (PANTHOR_DEV_QUERY, drm_panthor_dev_query, ioctl::RENDER_ALLOW, TyrDrmFileData::dev_query),
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index adbafe8db54d..e121303d88f0 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -80,6 +80,16 @@ pub struct Device<T: drm::Driver> {
> }
>
> impl<T: drm::Driver> Device<T> {
> + const fn compute_features() -> u32 {
> + let mut features = drm::driver::FEAT_GEM;
> +
> + if T::RENDER_CAPABILITY {
> + features |= drm::driver::FEAT_RENDER;
> + }
> +
> + features
> + }
> +
> const VTABLE: bindings::drm_driver = drm_legacy_fields! {
> load: None,
> open: Some(drm::File::<T::File>::open_callback),
> @@ -105,7 +115,7 @@ impl<T: drm::Driver> Device<T> {
> name: crate::str::as_char_ptr_in_const_context(T::INFO.name).cast_mut(),
> desc: crate::str::as_char_ptr_in_const_context(T::INFO.desc).cast_mut(),
>
> - driver_features: drm::driver::FEAT_GEM,
> + driver_features: Self::compute_features(),
> ioctls: T::IOCTLS.as_ptr(),
> num_ioctls: T::IOCTLS.len() as i32,
> fops: &Self::GEM_FOPS,
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index 5233bdebc9fc..92cbc26ce11f 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
> @@ -16,6 +16,8 @@
>
> /// Driver use the GEM memory manager. This should be set for all modern drivers.
> pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
> +/// Driver supports render nodes, i.e.: /dev/dri/renderDXX devices.
> +pub(crate) const FEAT_RENDER: u32 = bindings::drm_driver_feature_DRIVER_RENDER;
>
> /// Information data for a DRM Driver.
> pub struct DriverInfo {
> @@ -115,6 +117,9 @@ pub trait Driver {
>
> /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`.
> const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor];
> +
> + /// Sets the `DRIVER_RENDER` feature for this driver.
> + const RENDER_CAPABILITY: bool = false;
I think it would be nice for this documentation to elaborate more on
what this feature actually does. After all, it clearly took us a while
to understand it, so probably others are confused too.
Something along these lines:
/// Sets the `DRIVER_RENDER` feature for this driver.
///
/// When enabled, the driver exposes `/dev/dri/renderDXX` render nodes to
/// userspace. The render node is an alternate low-priviledge way to access
/// the driver, which is enforced on a per-ioctl level. Userspace processes
/// that open the render node can only invoke ioctls explicitly listed as
/// usable from the render node, whereas userspace processes using the
/// master node can invoke any ioctl.
const RENDER_CAPABILITY: bool = false;
Also, I'd probably call this const `FEAT_RENDER` for consistency / to
make it show up in grep.
Alice
next prev parent reply other threads:[~2026-05-05 10:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-05 9:23 [PATCH v2 0/1] DRM 'feature' support for DRM drivers Laura Nao
2026-05-05 9:23 ` [PATCH v2 1/1] rust: drm: add RENDER_CAPABILITY flag for render node support Laura Nao
2026-05-05 10:54 ` Alice Ryhl [this message]
2026-05-05 10:54 ` Alice Ryhl
2026-05-06 8:41 ` Laura Nao
2026-05-06 8:41 ` Laura Nao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afnMTfVfGq_iLXov@google.com \
--to=aliceryhl@google.com \
--cc=a.hindborg@kernel.org \
--cc=airlied@gmail.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=boris.brezillon@collabora.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=kernel@collabora.com \
--cc=laura.nao@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.