* [PATCH v2] drm/tyr: Clarify driver/device type names
@ 2026-02-23 20:46 Deborah Brouwer
2026-02-23 21:41 ` Danilo Krummrich
0 siblings, 1 reply; 3+ messages in thread
From: Deborah Brouwer @ 2026-02-23 20:46 UTC (permalink / raw)
To: dri-devel, rust-for-linux
Cc: daniel.almeida, aliceryhl, boris.brezillon, deborah.brouwer
Currently the `TyrDriver` struct implements both `platform::Driver` and
`drm::Driver`. For clarity, split up these two roles:
- Introduce `TyrPlatformDeviceData` to implement `platform::Driver`, and
- Introduce `TyrDrmDriver` to implement `drm::Driver`.
Also rename other variables to reflect their roles in the DRM context:
- Rename `TyrDevice` to `TyrDrmDevice`
- Rename `TyrData` to `TyrDrmDeviceData`
- Rename `File` to `TyrDrmFileData`
- Rename `DrmFile` to `TyrDrmFile`
No functional changes are intended.
Co-developed-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
---
Changes in v2:
- Move the import TyrDrmDriver to a single line.
- Rename the variable `tdev` -> `ddev` in file.rs to be
consistent with the name used in driver.rs.
Link to v1: https://lore.kernel.org/rust-for-linux/20260205185906.40940-1-deborah.brouwer@collabora.com/
This patch depends on:
[PATCH v2] drm/tyr: Use vertical style for imports
https://lore.kernel.org/rust-for-linux/20260223203833.207955-1-deborah.brouwer@collabora.com/
drivers/gpu/drm/tyr/driver.rs | 40 ++++++++++++++++++-----------------
drivers/gpu/drm/tyr/file.rs | 23 +++++++++-----------
drivers/gpu/drm/tyr/gem.rs | 9 +++-----
drivers/gpu/drm/tyr/tyr.rs | 4 ++--
4 files changed, 36 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 259a5157eb47..7a42e5d96d82 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -30,7 +30,7 @@
};
use crate::{
- file::File,
+ file::TyrDrmFileData,
gem::TyrObject,
gpu,
gpu::GpuInfo,
@@ -39,16 +39,18 @@
pub(crate) type IoMem = kernel::io::mem::IoMem<SZ_2M>;
+pub(crate) struct TyrDrmDriver;
+
/// Convenience type alias for the DRM device type for this driver.
-pub(crate) type TyrDevice = drm::Device<TyrDriver>;
+pub(crate) type TyrDrmDevice = drm::Device<TyrDrmDriver>;
#[pin_data(PinnedDrop)]
-pub(crate) struct TyrDriver {
- _device: ARef<TyrDevice>,
+pub(crate) struct TyrPlatformDeviceData {
+ _device: ARef<TyrDrmDevice>,
}
#[pin_data(PinnedDrop)]
-pub(crate) struct TyrData {
+pub(crate) struct TyrDrmDeviceData {
pub(crate) pdev: ARef<platform::Device>,
#[pin]
@@ -71,9 +73,9 @@ pub(crate) struct TyrData {
// that it will be removed in a future patch.
//
// SAFETY: This will be removed in a future patch.
-unsafe impl Send for TyrData {}
+unsafe impl Send for TyrDrmDeviceData {}
// SAFETY: This will be removed in a future patch.
-unsafe impl Sync for TyrData {}
+unsafe impl Sync for TyrDrmDeviceData {}
fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
regs::GPU_CMD.write(dev, iomem, regs::GPU_CMD_SOFT_RESET)?;
@@ -92,14 +94,14 @@ fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
kernel::of_device_table!(
OF_TABLE,
MODULE_OF_TABLE,
- <TyrDriver as platform::Driver>::IdInfo,
+ <TyrPlatformDeviceData as platform::Driver>::IdInfo,
[
(of::DeviceId::new(c"rockchip,rk3588-mali"), ()),
(of::DeviceId::new(c"arm,mali-valhall-csf"), ())
]
);
-impl platform::Driver for TyrDriver {
+impl platform::Driver for TyrPlatformDeviceData {
type IdInfo = ();
const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
@@ -129,7 +131,7 @@ fn probe(
let platform: ARef<platform::Device> = pdev.into();
- let data = try_pin_init!(TyrData {
+ let data = try_pin_init!(TyrDrmDeviceData {
pdev: platform.clone(),
clks <- new_mutex!(Clocks {
core: core_clk,
@@ -143,10 +145,10 @@ fn probe(
gpu_info,
});
- let tdev: ARef<TyrDevice> = drm::Device::new(pdev.as_ref(), data)?;
- drm::driver::Registration::new_foreign_owned(&tdev, pdev.as_ref(), 0)?;
+ let ddev: ARef<TyrDrmDevice> = drm::Device::new(pdev.as_ref(), data)?;
+ drm::driver::Registration::new_foreign_owned(&ddev, pdev.as_ref(), 0)?;
- let driver = TyrDriver { _device: tdev };
+ let driver = TyrPlatformDeviceData { _device: ddev };
// We need this to be dev_info!() because dev_dbg!() does not work at
// all in Rust for now, and we need to see whether probe succeeded.
@@ -156,12 +158,12 @@ fn probe(
}
#[pinned_drop]
-impl PinnedDrop for TyrDriver {
+impl PinnedDrop for TyrPlatformDeviceData {
fn drop(self: Pin<&mut Self>) {}
}
#[pinned_drop]
-impl PinnedDrop for TyrData {
+impl PinnedDrop for TyrDrmDeviceData {
fn drop(self: Pin<&mut Self>) {
// TODO: the type-state pattern for Clks will fix this.
let clks = self.clks.lock();
@@ -182,15 +184,15 @@ fn drop(self: Pin<&mut Self>) {
};
#[vtable]
-impl drm::Driver for TyrDriver {
- type Data = TyrData;
- type File = File;
+impl drm::Driver for TyrDrmDriver {
+ type Data = TyrDrmDeviceData;
+ type File = TyrDrmFileData;
type Object = drm::gem::Object<TyrObject>;
const INFO: drm::DriverInfo = INFO;
kernel::declare_drm_ioctls! {
- (PANTHOR_DEV_QUERY, drm_panthor_dev_query, ioctl::RENDER_ALLOW, File::dev_query),
+ (PANTHOR_DEV_QUERY, drm_panthor_dev_query, ioctl::RENDER_ALLOW, TyrDrmFileData::dev_query),
}
}
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 48bff4476d74..450be5ab9aaf 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -7,35 +7,32 @@
uapi, //
};
-use crate::{
- driver::TyrDevice,
- TyrDriver, //
-};
+use crate::driver::TyrDrmDriver;
#[pin_data]
-pub(crate) struct File {}
+pub(crate) struct TyrDrmFileData {}
/// Convenience type alias for our DRM `File` type
-pub(crate) type DrmFile = drm::file::File<File>;
+pub(crate) type TyrDrmFile = drm::file::File<TyrDrmFileData>;
-impl drm::file::DriverFile for File {
- type Driver = TyrDriver;
+impl drm::file::DriverFile for TyrDrmFileData {
+ type Driver = TyrDrmDriver;
fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
KBox::try_pin_init(try_pin_init!(Self {}), GFP_KERNEL)
}
}
-impl File {
+impl TyrDrmFileData {
pub(crate) fn dev_query(
- tdev: &TyrDevice,
+ ddev: &drm::Device<TyrDrmDriver>,
devquery: &mut uapi::drm_panthor_dev_query,
- _file: &DrmFile,
+ _file: &TyrDrmFile,
) -> Result<u32> {
if devquery.pointer == 0 {
match devquery.type_ {
uapi::drm_panthor_dev_query_type_DRM_PANTHOR_DEV_QUERY_GPU_INFO => {
- devquery.size = core::mem::size_of_val(&tdev.gpu_info) as u32;
+ devquery.size = core::mem::size_of_val(&ddev.gpu_info) as u32;
Ok(0)
}
_ => Err(EINVAL),
@@ -49,7 +46,7 @@ pub(crate) fn dev_query(
)
.writer();
- writer.write(&tdev.gpu_info)?;
+ writer.write(&ddev.gpu_info)?;
Ok(0)
}
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 8f2d23e3c093..514524ae07ef 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -5,19 +5,16 @@
prelude::*, //
};
-use crate::driver::{
- TyrDevice,
- TyrDriver, //
-};
+use crate::driver::TyrDrmDriver;
/// GEM Object inner driver data
#[pin_data]
pub(crate) struct TyrObject {}
impl gem::DriverObject for TyrObject {
- type Driver = TyrDriver;
+ type Driver = TyrDrmDriver;
- fn new(_dev: &TyrDevice, _size: usize) -> impl PinInit<Self, Error> {
+ fn new(_dev: &kernel::drm::Device<TyrDrmDriver>, _size: usize) -> impl PinInit<Self, Error> {
try_pin_init!(TyrObject {})
}
}
diff --git a/drivers/gpu/drm/tyr/tyr.rs b/drivers/gpu/drm/tyr/tyr.rs
index 861d1db43072..6eaa2135fe07 100644
--- a/drivers/gpu/drm/tyr/tyr.rs
+++ b/drivers/gpu/drm/tyr/tyr.rs
@@ -5,7 +5,7 @@
//! The name "Tyr" is inspired by Norse mythology, reflecting Arm's tradition of
//! naming their GPUs after Nordic mythological figures and places.
-use crate::driver::TyrDriver;
+use crate::driver::TyrPlatformDeviceData;
mod driver;
mod file;
@@ -14,7 +14,7 @@
mod regs;
kernel::module_platform_driver! {
- type: TyrDriver,
+ type: TyrPlatformDeviceData,
name: "tyr",
authors: ["The Tyr driver authors"],
description: "Arm Mali Tyr DRM driver",
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] drm/tyr: Clarify driver/device type names
2026-02-23 20:46 [PATCH v2] drm/tyr: Clarify driver/device type names Deborah Brouwer
@ 2026-02-23 21:41 ` Danilo Krummrich
2026-02-24 0:01 ` Deborah Brouwer
0 siblings, 1 reply; 3+ messages in thread
From: Danilo Krummrich @ 2026-02-23 21:41 UTC (permalink / raw)
To: Deborah Brouwer
Cc: dri-devel, rust-for-linux, daniel.almeida, aliceryhl,
boris.brezillon, dri-devel
On Mon Feb 23, 2026 at 9:46 PM CET, Deborah Brouwer wrote:
> Currently the `TyrDriver` struct implements both `platform::Driver` and
> `drm::Driver`. For clarity, split up these two roles:
> - Introduce `TyrPlatformDeviceData` to implement `platform::Driver`, and
I think this should be TyrPlatformDriverData instead, i.e. it is the driver's
private data that happens to be stored in (and owned by) the platform device.
The corresponding C setter is also called platform_set_drvdata() and not
platform_set_devdata().
> - Introduce `TyrDrmDriver` to implement `drm::Driver`.
>
> Also rename other variables to reflect their roles in the DRM context:
> - Rename `TyrDevice` to `TyrDrmDevice`
> - Rename `TyrData` to `TyrDrmDeviceData`
Although it might sound contradictory at first, I think for a drm::Device it is
OK to name it TyrDrmDeviceData as opposed to TyrDrmDriverData (although that
would make sense as well and I'd probably prefer that).
The reason is that a platform::Device being a bus device may store different
*driver specific* data types thoughout its whole lifetime, i.e. a platform
device may be bound to multiple different drivers throughout its lifetime.
But a drm::Device being a class device is always *statically* typed over it's
private data type, i.e. drm::Device<TyrDrmDeviceData>.
> - Rename `File` to `TyrDrmFileData`
> - Rename `DrmFile` to `TyrDrmFile`
>
> No functional changes are intended.
>
> Co-developed-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
Otherwise LGTM.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/tyr: Clarify driver/device type names
2026-02-23 21:41 ` Danilo Krummrich
@ 2026-02-24 0:01 ` Deborah Brouwer
0 siblings, 0 replies; 3+ messages in thread
From: Deborah Brouwer @ 2026-02-24 0:01 UTC (permalink / raw)
To: Danilo Krummrich
Cc: dri-devel, rust-for-linux, daniel.almeida, aliceryhl,
boris.brezillon, dri-devel
On Mon, Feb 23, 2026 at 10:41:04PM +0100, Danilo Krummrich wrote:
> On Mon Feb 23, 2026 at 9:46 PM CET, Deborah Brouwer wrote:
> > Currently the `TyrDriver` struct implements both `platform::Driver` and
> > `drm::Driver`. For clarity, split up these two roles:
> > - Introduce `TyrPlatformDeviceData` to implement `platform::Driver`, and
>
> I think this should be TyrPlatformDriverData instead, i.e. it is the driver's
> private data that happens to be stored in (and owned by) the platform device.
>
> The corresponding C setter is also called platform_set_drvdata() and not
> platform_set_devdata().
Yeah, I think you’re right, since we use this struct for
platform::Driver. I’ll rename it and send a v3.
>
> > - Introduce `TyrDrmDriver` to implement `drm::Driver`.
> >
> > Also rename other variables to reflect their roles in the DRM context:
> > - Rename `TyrDevice` to `TyrDrmDevice`
> > - Rename `TyrData` to `TyrDrmDeviceData`
>
> Although it might sound contradictory at first, I think for a drm::Device it is
> OK to name it TyrDrmDeviceData as opposed to TyrDrmDriverData (although that
> would make sense as well and I'd probably prefer that).
>
> The reason is that a platform::Device being a bus device may store different
> *driver specific* data types thoughout its whole lifetime, i.e. a platform
> device may be bound to multiple different drivers throughout its lifetime.
>
> But a drm::Device being a class device is always *statically* typed over it's
> private data type, i.e. drm::Device<TyrDrmDeviceData>.
I'd definitely like to keep TyrDrmDeviceData for this struct because of
its relationship to the drm::Device.
Thanks for your review.
>
> > - Rename `File` to `TyrDrmFileData`
> > - Rename `DrmFile` to `TyrDrmFile`
> >
> > No functional changes are intended.
> >
> > Co-developed-by: Boris Brezillon <boris.brezillon@collabora.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
>
> Otherwise LGTM.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-02-24 0:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-23 20:46 [PATCH v2] drm/tyr: Clarify driver/device type names Deborah Brouwer
2026-02-23 21:41 ` Danilo Krummrich
2026-02-24 0:01 ` Deborah Brouwer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox