From 461b8d67065717aa049e99ff45d19a9f692db4d2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Dec 2025 10:40:30 +0100 Subject: [PATCH 1/6] drivers/graphics: Introduce DrmObject trait to reduce code duplication --- drivers/graphics/driver-graphics/src/lib.rs | 2 +- .../graphics/driver-graphics/src/objects.rs | 230 +++++++++--------- 2 files changed, 117 insertions(+), 115 deletions(-) diff --git a/drivers/graphics/driver-graphics/src/lib.rs b/drivers/graphics/driver-graphics/src/lib.rs index b77af3e4d9..2822e93486 100644 --- a/drivers/graphics/driver-graphics/src/lib.rs +++ b/drivers/graphics/driver-graphics/src/lib.rs @@ -35,7 +35,7 @@ pub struct StandardProperties { } pub trait GraphicsAdapter: Sized + Debug { - type Connector: Debug; + type Connector: Debug + 'static; type Framebuffer: Framebuffer; type Cursor: CursorFramebuffer; diff --git a/drivers/graphics/driver-graphics/src/objects.rs b/drivers/graphics/driver-graphics/src/objects.rs index 3cd6ea3730..4dd449e783 100644 --- a/drivers/graphics/driver-graphics/src/objects.rs +++ b/drivers/graphics/driver-graphics/src/objects.rs @@ -1,5 +1,8 @@ +use std::any::Any; use std::collections::HashMap; use std::ffi::c_char; +use std::fmt::Debug; +use std::marker::PhantomData; use drm_sys::{ drm_mode_modeinfo, DRM_MODE_OBJECT_BLOB, DRM_MODE_OBJECT_CONNECTOR, DRM_MODE_OBJECT_ENCODER, @@ -14,7 +17,8 @@ pub struct DrmObjects { next_id: DrmObjectId, connectors: Vec, encoders: Vec, - objects: HashMap>, + objects: HashMap, + _marker: PhantomData, } impl DrmObjects { @@ -24,17 +28,45 @@ impl DrmObjects { connectors: vec![], encoders: vec![], objects: HashMap::new(), + _marker: PhantomData, + } + } + + fn add(&mut self, data: U) -> DrmObjectId { + let id = self.next_id; + self.objects.insert( + id, + DrmObjectData { + kind: Box::new(data), + properties: vec![], + }, + ); + self.next_id.0 += 1; + + id + } + + fn get(&self, id: DrmObjectId) -> Result<&U> { + let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; + if let Some(object) = (&*object.kind as &dyn Any).downcast_ref::() { + Ok(object) + } else { + Err(Error::new(EINVAL)) + } + } + + fn get_mut(&mut self, id: DrmObjectId) -> Result<&mut U> { + let object = self.objects.get_mut(&id).ok_or(Error::new(EINVAL))?; + if let Some(object) = (&mut *object.kind as &mut dyn Any).downcast_mut::() { + Ok(object) + } else { + Err(Error::new(EINVAL)) } } pub fn object_type(&self, id: DrmObjectId) -> Result { let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; - Ok(match object.kind { - DrmObjectKind::Property(_) => DRM_MODE_OBJECT_PROPERTY, - DrmObjectKind::Blob(_) => DRM_MODE_OBJECT_BLOB, - DrmObjectKind::Connector(_) => DRM_MODE_OBJECT_CONNECTOR, - DrmObjectKind::Encoder(_) => DRM_MODE_OBJECT_ENCODER, - }) + Ok(object.kind.object_type()) } pub fn add_property( @@ -76,30 +108,16 @@ impl DrmObjects { *to = from as c_char; } - let id = self.next_id; - self.objects.insert( - id, - DrmObject { - kind: DrmObjectKind::Property(DrmProperty { - name: name_bytes, - immutable, - atomic, - kind, - }), - properties: vec![], - }, - ); - self.next_id.0 += 1; - - id + self.add(DrmProperty { + name: name_bytes, + immutable, + atomic, + kind, + }) } pub fn get_property(&self, id: DrmObjectId) -> Result<&DrmProperty> { - let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; - match &object.kind { - DrmObjectKind::Property(drm_property) => Ok(drm_property), - _ => Err(Error::new(EINVAL)), - } + self.get(id) } pub fn add_object_property(&mut self, object: DrmObjectId, property: DrmObjectId, value: u64) { @@ -124,62 +142,35 @@ impl DrmObjects { } pub fn add_blob(&mut self, data: Vec) -> DrmObjectId { - let id = self.next_id; - self.objects.insert( - id, - DrmObject { - kind: DrmObjectKind::Blob(data), - properties: vec![], - }, - ); - self.next_id.0 += 1; - - id + self.add(DrmBlob { data }) } pub fn get_blob(&self, id: DrmObjectId) -> Result<&[u8]> { - let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; - match &object.kind { - DrmObjectKind::Blob(data) => Ok(data), - _ => Err(Error::new(EINVAL)), - } + Ok(&self.get::(id)?.data) } pub fn add_connector(&mut self, driver_data: T::Connector) -> DrmObjectId { - let connector_id = self.next_id; - let encoder_id = DrmObjectId(self.next_id.0 + 1); - self.objects.insert( - connector_id, - DrmObject { - kind: DrmObjectKind::Connector(DrmConnector { - modes: vec![], - encoder_id, - connector_type: 0, - connector_type_id: 0, - connection: DrmConnectorStatus::Unknown, - mm_width: 0, - mm_height: 0, - subpixel: DrmSubpixelOrder::Unknown, - driver_data, - }), - properties: vec![], - }, - ); + let connector_id = self.add(DrmConnector { + modes: vec![], + encoder_id: DrmObjectId::INVALID, + connector_type: 0, + connector_type_id: 0, + connection: DrmConnectorStatus::Unknown, + mm_width: 0, + mm_height: 0, + subpixel: DrmSubpixelOrder::Unknown, + driver_data, + }); self.connectors.push(connector_id); - self.objects.insert( - encoder_id, - DrmObject { - kind: DrmObjectKind::Encoder(DrmEncoder { - crtc_id: DrmObjectId::INVALID, - possible_crtcs: 0, - possible_clones: 0, - }), - properties: vec![], - }, - ); + let encoder_id = self.add(DrmEncoder { + crtc_id: DrmObjectId::INVALID, + possible_crtcs: 0, + possible_clones: 0, + }); self.encoders.push(encoder_id); - self.next_id.0 += 2; + + self.get_connector_mut(connector_id).unwrap().encoder_id = encoder_id; connector_id } @@ -188,29 +179,23 @@ impl DrmObjects { &self.connectors } - pub fn connectors(&self) -> impl Iterator> + use<'_, T> { - self.connectors - .iter() - .map(|&id| match &self.objects[&id].kind { - DrmObjectKind::Connector(connector) => connector, - _ => unreachable!(), - }) + pub fn connectors(&self) -> impl Iterator> + use<'_, T> { + self.connectors.iter().map(|&id| { + (&self.objects[&id].kind as &dyn Any) + .downcast_ref::>() + .unwrap() + }) } - pub fn get_connector(&self, id: DrmObjectId) -> Result<&DrmConnector> { - let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; - match &object.kind { - DrmObjectKind::Connector(drm_connector) => Ok(drm_connector), - _ => Err(Error::new(EINVAL)), - } + pub fn get_connector(&self, id: DrmObjectId) -> Result<&DrmConnector> { + self.get(id) } - pub fn get_connector_mut(&mut self, id: DrmObjectId) -> Result<&mut DrmConnector> { - let object = self.objects.get_mut(&id).ok_or(Error::new(EINVAL))?; - match &mut object.kind { - DrmObjectKind::Connector(drm_connector) => Ok(drm_connector), - _ => Err(Error::new(EINVAL)), - } + pub fn get_connector_mut( + &mut self, + id: DrmObjectId, + ) -> Result<&mut DrmConnector> { + self.get_mut(id) } pub fn encoder_ids(&self) -> &[DrmObjectId] { @@ -218,19 +203,11 @@ impl DrmObjects { } pub fn get_encoder(&self, id: DrmObjectId) -> Result<&DrmEncoder> { - let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; - match &object.kind { - DrmObjectKind::Encoder(drm_encoder) => Ok(drm_encoder), - _ => Err(Error::new(EINVAL)), - } + self.get(id) } pub fn get_encoder_mut(&mut self, id: DrmObjectId) -> Result<&mut DrmEncoder> { - let object = self.objects.get_mut(&id).ok_or(Error::new(EINVAL))?; - match &mut object.kind { - DrmObjectKind::Encoder(drm_encoder) => Ok(drm_encoder), - _ => Err(Error::new(EINVAL)), - } + self.get_mut(id) } } @@ -248,17 +225,13 @@ impl From for u64 { } #[derive(Debug)] -struct DrmObject { - kind: DrmObjectKind, +struct DrmObjectData { + kind: Box, properties: Vec<(DrmObjectId, u64)>, } -#[derive(Debug)] -enum DrmObjectKind { - Property(DrmProperty), - Blob(Vec), - Connector(DrmConnector), - Encoder(DrmEncoder), +pub trait DrmObject: Any + Debug { + fn object_type(&self) -> u32; } #[derive(Debug)] @@ -279,8 +252,25 @@ pub enum DrmPropertyKind { SignedRange(i64, i64), } +impl DrmObject for DrmProperty { + fn object_type(&self) -> u32 { + DRM_MODE_OBJECT_PROPERTY + } +} + #[derive(Debug)] -pub struct DrmConnector { +pub struct DrmBlob { + data: Vec, +} + +impl DrmObject for DrmBlob { + fn object_type(&self) -> u32 { + DRM_MODE_OBJECT_BLOB + } +} + +#[derive(Debug)] +pub struct DrmConnector { pub modes: Vec, pub encoder_id: DrmObjectId, pub connector_type: u32, @@ -289,7 +279,7 @@ pub struct DrmConnector { pub mm_width: u32, pub mm_height: u32, pub subpixel: DrmSubpixelOrder, - pub driver_data: T::Connector, + pub driver_data: T, } #[derive(Debug, Copy, Clone)] @@ -311,6 +301,12 @@ pub enum DrmSubpixelOrder { None, } +impl DrmObject for DrmConnector { + fn object_type(&self) -> u32 { + DRM_MODE_OBJECT_CONNECTOR + } +} + // FIXME can we represent connector and encoder using a single struct? #[derive(Debug)] pub struct DrmEncoder { @@ -318,3 +314,9 @@ pub struct DrmEncoder { pub possible_crtcs: u32, pub possible_clones: u32, } + +impl DrmObject for DrmEncoder { + fn object_type(&self) -> u32 { + DRM_MODE_OBJECT_ENCODER + } +} From d5cf3db42863fc0863e40906110c91e2f5952658 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Dec 2025 10:44:54 +0100 Subject: [PATCH 2/6] drivers/graphics: Move handling of properties to a separate file --- drivers/graphics/driver-graphics/src/lib.rs | 4 +- .../graphics/driver-graphics/src/objects.rs | 133 +----------------- .../driver-graphics/src/properties.rs | 125 ++++++++++++++++ 3 files changed, 135 insertions(+), 127 deletions(-) create mode 100644 drivers/graphics/driver-graphics/src/properties.rs diff --git a/drivers/graphics/driver-graphics/src/lib.rs b/drivers/graphics/driver-graphics/src/lib.rs index 2822e93486..ada783988e 100644 --- a/drivers/graphics/driver-graphics/src/lib.rs +++ b/drivers/graphics/driver-graphics/src/lib.rs @@ -24,9 +24,11 @@ use redox_scheme::{CallerCtx, OpenResult, RequestKind, SignalBehavior, Socket}; use syscall::schemev2::NewFdFlags; use syscall::{Error, MapFlags, Result, EAGAIN, EBADF, EINVAL, ENOENT, EOPNOTSUPP}; -use crate::objects::{DrmObjectId, DrmObjects, DrmPropertyKind}; +use crate::objects::{DrmObjectId, DrmObjects}; +use crate::properties::DrmPropertyKind; pub mod objects; +pub mod properties; #[derive(Debug, Copy, Clone)] pub struct StandardProperties { diff --git a/drivers/graphics/driver-graphics/src/objects.rs b/drivers/graphics/driver-graphics/src/objects.rs index 4dd449e783..a90029e374 100644 --- a/drivers/graphics/driver-graphics/src/objects.rs +++ b/drivers/graphics/driver-graphics/src/objects.rs @@ -1,13 +1,9 @@ use std::any::Any; use std::collections::HashMap; -use std::ffi::c_char; use std::fmt::Debug; use std::marker::PhantomData; -use drm_sys::{ - drm_mode_modeinfo, DRM_MODE_OBJECT_BLOB, DRM_MODE_OBJECT_CONNECTOR, DRM_MODE_OBJECT_ENCODER, - DRM_MODE_OBJECT_PROPERTY, DRM_PROP_NAME_LEN, -}; +use drm_sys::{drm_mode_modeinfo, DRM_MODE_OBJECT_CONNECTOR, DRM_MODE_OBJECT_ENCODER}; use syscall::{Error, Result, EINVAL}; use crate::GraphicsAdapter; @@ -17,7 +13,7 @@ pub struct DrmObjects { next_id: DrmObjectId, connectors: Vec, encoders: Vec, - objects: HashMap, + pub(crate) objects: HashMap, _marker: PhantomData, } @@ -32,7 +28,7 @@ impl DrmObjects { } } - fn add(&mut self, data: U) -> DrmObjectId { + pub(crate) fn add(&mut self, data: U) -> DrmObjectId { let id = self.next_id; self.objects.insert( id, @@ -46,7 +42,7 @@ impl DrmObjects { id } - fn get(&self, id: DrmObjectId) -> Result<&U> { + pub(crate) fn get(&self, id: DrmObjectId) -> Result<&U> { let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; if let Some(object) = (&*object.kind as &dyn Any).downcast_ref::() { Ok(object) @@ -55,7 +51,7 @@ impl DrmObjects { } } - fn get_mut(&mut self, id: DrmObjectId) -> Result<&mut U> { + pub(crate) fn get_mut(&mut self, id: DrmObjectId) -> Result<&mut U> { let object = self.objects.get_mut(&id).ok_or(Error::new(EINVAL))?; if let Some(object) = (&mut *object.kind as &mut dyn Any).downcast_mut::() { Ok(object) @@ -69,86 +65,6 @@ impl DrmObjects { Ok(object.kind.object_type()) } - pub fn add_property( - &mut self, - name: &str, - immutable: bool, - atomic: bool, - kind: DrmPropertyKind, - ) -> DrmObjectId { - if name.len() > DRM_PROP_NAME_LEN as usize { - panic!("Property name {name} is too long"); - } - - match &kind { - DrmPropertyKind::Range(start, end) => assert!(start < end), - DrmPropertyKind::Enum(variants) => { - // FIXME check duplicate variant numbers - for (variant_name, _) in variants { - if variant_name.len() > DRM_PROP_NAME_LEN as usize { - panic!("Property variant name {variant_name} is too long"); - } - } - } - DrmPropertyKind::Blob => {} - DrmPropertyKind::Bitmask(bitmask_flags) => { - // FIXME check overlapping flag numbers - for (flag_name, _) in bitmask_flags { - if flag_name.len() > DRM_PROP_NAME_LEN as usize { - panic!("Property bitflag name {flag_name} is too long"); - } - } - } - DrmPropertyKind::Object => {} - DrmPropertyKind::SignedRange(start, end) => assert!(start < end), - } - - let mut name_bytes = [0; DRM_PROP_NAME_LEN as usize]; - for (to, &from) in name_bytes.iter_mut().zip(name.as_bytes()) { - *to = from as c_char; - } - - self.add(DrmProperty { - name: name_bytes, - immutable, - atomic, - kind, - }) - } - - pub fn get_property(&self, id: DrmObjectId) -> Result<&DrmProperty> { - self.get(id) - } - - pub fn add_object_property(&mut self, object: DrmObjectId, property: DrmObjectId, value: u64) { - let object = self.objects.get_mut(&object).unwrap(); - // FIXME validate property uniqueness and value - object.properties.push((property, value)); - } - - pub fn set_object_property(&mut self, object: DrmObjectId, property: DrmObjectId, value: u64) { - let object = self.objects.get_mut(&object).unwrap(); - // FIXME validate property existence and value - for (prop, val) in object.properties.iter_mut() { - if *prop == property { - *val = value; - } - } - } - - pub fn get_object_properties(&self, id: DrmObjectId) -> Result<&[(DrmObjectId, u64)]> { - let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; - Ok(&object.properties) - } - - pub fn add_blob(&mut self, data: Vec) -> DrmObjectId { - self.add(DrmBlob { data }) - } - - pub fn get_blob(&self, id: DrmObjectId) -> Result<&[u8]> { - Ok(&self.get::(id)?.data) - } - pub fn add_connector(&mut self, driver_data: T::Connector) -> DrmObjectId { let connector_id = self.add(DrmConnector { modes: vec![], @@ -225,50 +141,15 @@ impl From for u64 { } #[derive(Debug)] -struct DrmObjectData { +pub(crate) struct DrmObjectData { kind: Box, - properties: Vec<(DrmObjectId, u64)>, + pub(crate) properties: Vec<(DrmObjectId, u64)>, } pub trait DrmObject: Any + Debug { fn object_type(&self) -> u32; } -#[derive(Debug)] -pub struct DrmProperty { - pub name: [c_char; DRM_PROP_NAME_LEN as usize], - pub immutable: bool, - pub atomic: bool, - pub kind: DrmPropertyKind, -} - -#[derive(Debug)] -pub enum DrmPropertyKind { - Range(u64, u64), - Enum(Vec<(&'static str, u64)>), - Blob, - Bitmask(Vec<(&'static str, u64)>), - Object, - SignedRange(i64, i64), -} - -impl DrmObject for DrmProperty { - fn object_type(&self) -> u32 { - DRM_MODE_OBJECT_PROPERTY - } -} - -#[derive(Debug)] -pub struct DrmBlob { - data: Vec, -} - -impl DrmObject for DrmBlob { - fn object_type(&self) -> u32 { - DRM_MODE_OBJECT_BLOB - } -} - #[derive(Debug)] pub struct DrmConnector { pub modes: Vec, diff --git a/drivers/graphics/driver-graphics/src/properties.rs b/drivers/graphics/driver-graphics/src/properties.rs new file mode 100644 index 0000000000..4ce9169135 --- /dev/null +++ b/drivers/graphics/driver-graphics/src/properties.rs @@ -0,0 +1,125 @@ +use std::ffi::c_char; +use std::fmt::Debug; + +use drm_sys::{DRM_MODE_OBJECT_BLOB, DRM_MODE_OBJECT_PROPERTY, DRM_PROP_NAME_LEN}; +use syscall::{Error, Result, EINVAL}; + +use crate::objects::{DrmObject, DrmObjectId, DrmObjects}; +use crate::GraphicsAdapter; + +impl DrmObjects { + pub fn add_property( + &mut self, + name: &str, + immutable: bool, + atomic: bool, + kind: DrmPropertyKind, + ) -> DrmObjectId { + if name.len() > DRM_PROP_NAME_LEN as usize { + panic!("Property name {name} is too long"); + } + + match &kind { + DrmPropertyKind::Range(start, end) => assert!(start < end), + DrmPropertyKind::Enum(variants) => { + // FIXME check duplicate variant numbers + for (variant_name, _) in variants { + if variant_name.len() > DRM_PROP_NAME_LEN as usize { + panic!("Property variant name {variant_name} is too long"); + } + } + } + DrmPropertyKind::Blob => {} + DrmPropertyKind::Bitmask(bitmask_flags) => { + // FIXME check overlapping flag numbers + for (flag_name, _) in bitmask_flags { + if flag_name.len() > DRM_PROP_NAME_LEN as usize { + panic!("Property bitflag name {flag_name} is too long"); + } + } + } + DrmPropertyKind::Object => {} + DrmPropertyKind::SignedRange(start, end) => assert!(start < end), + } + + let mut name_bytes = [0; DRM_PROP_NAME_LEN as usize]; + for (to, &from) in name_bytes.iter_mut().zip(name.as_bytes()) { + *to = from as c_char; + } + + self.add(DrmProperty { + name: name_bytes, + immutable, + atomic, + kind, + }) + } + + pub fn get_property(&self, id: DrmObjectId) -> Result<&DrmProperty> { + self.get(id) + } + + pub fn add_object_property(&mut self, object: DrmObjectId, property: DrmObjectId, value: u64) { + let object = self.objects.get_mut(&object).unwrap(); + // FIXME validate property uniqueness and value + object.properties.push((property, value)); + } + + pub fn set_object_property(&mut self, object: DrmObjectId, property: DrmObjectId, value: u64) { + let object = self.objects.get_mut(&object).unwrap(); + // FIXME validate property existence and value + for (prop, val) in object.properties.iter_mut() { + if *prop == property { + *val = value; + } + } + } + + pub fn get_object_properties(&self, id: DrmObjectId) -> Result<&[(DrmObjectId, u64)]> { + let object = self.objects.get(&id).ok_or(Error::new(EINVAL))?; + Ok(&object.properties) + } + + pub fn add_blob(&mut self, data: Vec) -> DrmObjectId { + self.add(DrmBlob { data }) + } + + pub fn get_blob(&self, id: DrmObjectId) -> Result<&[u8]> { + Ok(&self.get::(id)?.data) + } +} + +#[derive(Debug)] +pub struct DrmProperty { + pub name: [c_char; DRM_PROP_NAME_LEN as usize], + pub immutable: bool, + pub atomic: bool, + pub kind: DrmPropertyKind, +} + +#[derive(Debug)] +pub enum DrmPropertyKind { + Range(u64, u64), + Enum(Vec<(&'static str, u64)>), + Blob, + Bitmask(Vec<(&'static str, u64)>), + Object, + SignedRange(i64, i64), +} + +impl DrmObject for DrmProperty { + fn object_type(&self) -> u32 { + DRM_MODE_OBJECT_PROPERTY + } +} + +#[derive(Debug)] +pub struct DrmBlob { + data: Vec, +} + +impl DrmObject for DrmBlob { + fn object_type(&self) -> u32 { + DRM_MODE_OBJECT_BLOB + } +} From ff3cb3ce64f0cc59e6e7e49d988662ee855eed8a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Dec 2025 11:30:02 +0100 Subject: [PATCH 3/6] drivers/graphics/virtio-gpud: Poor attempt at parsing the EDID data --- Cargo.lock | 1 + drivers/graphics/virtio-gpud/Cargo.toml | 2 ++ drivers/graphics/virtio-gpud/src/scheme.rs | 37 +++++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 971472c4d2..bfcbc0d70b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2569,6 +2569,7 @@ dependencies = [ "daemon", "driver-graphics", "drm-sys", + "edid", "futures", "graphics-ipc", "inputd", diff --git a/drivers/graphics/virtio-gpud/Cargo.toml b/drivers/graphics/virtio-gpud/Cargo.toml index 50194c890d..ab34a0a258 100644 --- a/drivers/graphics/virtio-gpud/Cargo.toml +++ b/drivers/graphics/virtio-gpud/Cargo.toml @@ -6,6 +6,8 @@ authors = ["Anhad Singh "] [dependencies] drm-sys = "0.8.0" +#TODO: edid is abandoned, fork it an maintain? +edid = "0.3.0" log = "0.4" static_assertions = "1.1.0" futures = { version = "0.3.28", features = ["executor"] } diff --git a/drivers/graphics/virtio-gpud/src/scheme.rs b/drivers/graphics/virtio-gpud/src/scheme.rs index c9efd130b9..b8b4396626 100644 --- a/drivers/graphics/virtio-gpud/src/scheme.rs +++ b/drivers/graphics/virtio-gpud/src/scheme.rs @@ -7,7 +7,7 @@ use driver_graphics::{ modeinfo_for_size, CursorFramebuffer, CursorPlane, Framebuffer, GraphicsAdapter, GraphicsScheme, StandardProperties, }; -use drm_sys::DRM_MODE_DPMS_ON; +use drm_sys::{DRM_MODE_DPMS_ON, DRM_MODE_TYPE_PREFERRED}; use graphics_ipc::v1::Damage; use graphics_ipc::v2::ipc::{DRM_CAP_DUMB_BUFFER, DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT}; use inputd::DisplayHandle; @@ -307,6 +307,41 @@ impl<'a> GraphicsAdapter for VirtGpuAdapter<'a> { }; if self.has_edid { + let edid = edid::parse(&display.edid).unwrap().1; + + let first_detailed_timing = edid + .descriptors + .iter() + .find_map(|descriptor| match descriptor { + edid::Descriptor::DetailedTiming(detailed_timing) => Some(detailed_timing), + _ => None, + }) + .unwrap(); + connector.mm_width = first_detailed_timing.horizontal_size.into(); + connector.mm_height = first_detailed_timing.vertical_size.into(); + + connector.modes = edid + .descriptors + .iter() + .filter_map(|descriptor| { + match descriptor { + edid::Descriptor::DetailedTiming(detailed_timing) => { + // FIXME extract full information + Some(modeinfo_for_size( + u32::from(detailed_timing.horizontal_active_pixels), + u32::from(detailed_timing.vertical_active_lines), + )) + } + _ => None, + } + }) + .collect::>(); + + // First detailed timing descriptor indicates preferred mode. + for mode in connector.modes.iter_mut().skip(1) { + mode.flags &= !DRM_MODE_TYPE_PREFERRED; + } + let blob = objects.add_blob(display.edid.clone()); objects.set_object_property(id, standard_properties.edid, blob.into()); } From a4a0fbd1989cc0faf957693802f3e8c7ca5f480a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Dec 2025 11:48:47 +0100 Subject: [PATCH 4/6] drivers/graphics/ihdgd: Move edid probing into ddi.rs --- drivers/graphics/ihdgd/src/device/ddi.rs | 107 ++++++++++++++++++++ drivers/graphics/ihdgd/src/device/mod.rs | 118 ++--------------------- 2 files changed, 116 insertions(+), 109 deletions(-) diff --git a/drivers/graphics/ihdgd/src/device/ddi.rs b/drivers/graphics/ihdgd/src/device/ddi.rs index d5d3bb8ed7..67fb9cebb5 100644 --- a/drivers/graphics/ihdgd/src/device/ddi.rs +++ b/drivers/graphics/ihdgd/src/device/ddi.rs @@ -1,7 +1,14 @@ use common::io::{Io, MmioPtr, WriteOnly}; +use common::timeout::Timeout; +use embedded_hal::prelude::*; use std::sync::Arc; +use std::time::Duration; use syscall::error::{Error, Result, EIO}; +use crate::device::aux::Aux; +use crate::device::power::PowerWells; +use crate::device::{CallbackGuard, Gmbus, HalTimer}; + use super::{GpioPort, MmioRegion}; // IHD-OS-TGL-Vol 2c-12.21 DDI_AUX_CTL @@ -200,6 +207,106 @@ impl Ddi { self.port_reg((reg as usize) + (lane as usize)) } + pub fn probe_edid( + &mut self, + power_wells: &mut PowerWells, + gttmm: &MmioRegion, + gmbus: &mut Gmbus, + ) -> Result, Error> { + if let Some(port_comp_dw0) = self.port_comp(PortCompReg::Dw0) { + log::debug!("PORT_COMP_DW0_{}: {:08X}", self.name, port_comp_dw0.read()); + } + let mut aux_read_edid = |ddi: &mut Ddi| -> Result<[u8; 128]> { + //TODO: BLOCK TCCOLD? + + //TODO: the request can be shared by multiple DDIs + let pwr_well_ctl_aux_request = ddi.pwr_well_ctl_aux_request; + let pwr_well_ctl_aux_state = ddi.pwr_well_ctl_aux_state; + let mut pwr_well_ctl_aux = unsafe { MmioPtr::new(power_wells.ctl_aux.as_mut_ptr()) }; + let _pwr_guard = CallbackGuard::new( + &mut pwr_well_ctl_aux, + |pwr_well_ctl_aux| { + // Enable aux power + pwr_well_ctl_aux.writef(pwr_well_ctl_aux_request, true); + let timeout = Timeout::from_micros(1500); + while !pwr_well_ctl_aux.readf(pwr_well_ctl_aux_state) { + timeout.run().map_err(|()| { + log::debug!("timeout while requesting DDI {} aux power", ddi.name); + Error::new(EIO) + })?; + } + Ok(()) + }, + |pwr_well_ctl_aux| { + // Disable aux power + pwr_well_ctl_aux.writef(pwr_well_ctl_aux_request, false); + }, + )?; + + let mut edid_data = [0; 128]; + Aux::new(ddi) + .write_read(0x50, &[0x00], &mut edid_data) + .map_err(|_err| Error::new(EIO))?; + + Ok(edid_data) + }; + let mut gmbus_read_edid = |ddi: &mut Ddi| -> Result<[u8; 128]> { + let Some(pin_pair) = ddi.gmbus_pin_pair else { + return Err(Error::new(EIO)); + }; + + let mut edid_data = [0; 128]; + gmbus + .pin_pair(pin_pair) + .write_read(0x50, &[0x00], &mut edid_data) + .map_err(|_err| Error::new(EIO))?; + + Ok(edid_data) + }; + let gpio_read_edid = |ddi: &mut Ddi| -> Result<[u8; 128]> { + let Some(port) = &ddi.gpio_port else { + return Err(Error::new(EIO)); + }; + + let mut edid_data = [0; 128]; + let i2c_freq = 100_000.0; + bitbang_hal::i2c::I2cBB::new( + unsafe { port.clock(gttmm)? }, + unsafe { port.data(gttmm)? }, + HalTimer::new(Duration::from_secs_f64(1.0 / i2c_freq)), + ) + .write_read(0x50, &[0x00], &mut edid_data) + .map_err(|_err| Error::new(EIO))?; + + Ok(edid_data) + }; + let (source, edid_data) = match aux_read_edid(self) { + Ok(edid_data) => ("AUX", edid_data), + Err(err) => { + log::debug!("DDI {} failed to read EDID from AUX: {}", self.name, err); + match gmbus_read_edid(self) { + Ok(edid_data) => ("GMBUS", edid_data), + Err(err) => { + log::debug!("DDI {} failed to read EDID from GMBUS: {}", self.name, err); + match gpio_read_edid(self) { + Ok(edid_data) => ("GPIO", edid_data), + Err(err) => { + log::debug!( + "DDI {} failed to read EDID from GPIO: {}", + self.name, + err + ); + // Will try again but not fail the driver + return Ok(None); + } + } + } + } + } + }; + Ok(Some((source, edid_data))) + } + pub fn voltage_swing_hdmi( &mut self, gttmm: &MmioRegion, diff --git a/drivers/graphics/ihdgd/src/device/mod.rs b/drivers/graphics/ihdgd/src/device/mod.rs index 5a5dbb36dc..916afda64c 100644 --- a/drivers/graphics/ihdgd/src/device/mod.rs +++ b/drivers/graphics/ihdgd/src/device/mod.rs @@ -2,14 +2,12 @@ use common::{ io::{Io, MmioPtr}, timeout::Timeout, }; -use embedded_hal::prelude::*; use pcid_interface::{PciFunction, PciFunctionHandle}; use range_alloc::RangeAllocator; -use std::{collections::VecDeque, fmt, mem, sync::Arc, time::Duration}; +use std::{collections::VecDeque, fmt, mem, sync::Arc}; use syscall::error::{Error, Result, EIO, ENODEV, ERANGE}; mod aux; -use self::aux::*; mod bios; use self::bios::*; mod ddi; @@ -552,102 +550,10 @@ impl Device { // Enable DDI power well self.power_wells.enable_well_by_ddi(ddi.name)?; - //TODO: init port if needed - if let Some(port_comp_dw0) = ddi.port_comp(PortCompReg::Dw0) { - log::debug!("PORT_COMP_DW0_{}: {:08X}", ddi.name, port_comp_dw0.read()); - } - - let mut aux_read_edid = |ddi: &mut Ddi| -> Result<[u8; 128]> { - //TODO: BLOCK TCCOLD? - - //TODO: the request can be shared by multiple DDIs - let pwr_well_ctl_aux_request = ddi.pwr_well_ctl_aux_request; - let pwr_well_ctl_aux_state = ddi.pwr_well_ctl_aux_state; - let mut pwr_well_ctl_aux = - unsafe { MmioPtr::new(self.power_wells.ctl_aux.as_mut_ptr()) }; - let _pwr_guard = CallbackGuard::new( - &mut pwr_well_ctl_aux, - |pwr_well_ctl_aux| { - // Enable aux power - pwr_well_ctl_aux.writef(pwr_well_ctl_aux_request, true); - let timeout = Timeout::from_micros(1500); - while !pwr_well_ctl_aux.readf(pwr_well_ctl_aux_state) { - timeout.run().map_err(|()| { - log::debug!("timeout while requesting DDI {} aux power", ddi.name); - Error::new(EIO) - })?; - } - Ok(()) - }, - |pwr_well_ctl_aux| { - // Disable aux power - pwr_well_ctl_aux.writef(pwr_well_ctl_aux_request, false); - }, - )?; - - let mut edid_data = [0; 128]; - Aux::new(ddi) - .write_read(0x50, &[0x00], &mut edid_data) - .map_err(|_err| Error::new(EIO))?; - - Ok(edid_data) - }; - - let mut gmbus_read_edid = |ddi: &mut Ddi| -> Result<[u8; 128]> { - let Some(pin_pair) = ddi.gmbus_pin_pair else { - return Err(Error::new(EIO)); - }; - - let mut edid_data = [0; 128]; - self.gmbus - .pin_pair(pin_pair) - .write_read(0x50, &[0x00], &mut edid_data) - .map_err(|_err| Error::new(EIO))?; - - Ok(edid_data) - }; - - let gpio_read_edid = |ddi: &mut Ddi| -> Result<[u8; 128]> { - let Some(port) = &ddi.gpio_port else { - return Err(Error::new(EIO)); - }; - - let mut edid_data = [0; 128]; - let i2c_freq = 100_000.0; - bitbang_hal::i2c::I2cBB::new( - unsafe { port.clock(&self.gttmm)? }, - unsafe { port.data(&self.gttmm)? }, - HalTimer::new(Duration::from_secs_f64(1.0 / i2c_freq)), - ) - .write_read(0x50, &[0x00], &mut edid_data) - .map_err(|_err| Error::new(EIO))?; - - Ok(edid_data) - }; - - let (source, edid_data) = match aux_read_edid(ddi) { - Ok(edid_data) => ("AUX", edid_data), - Err(err) => { - log::debug!("DDI {} failed to read EDID from AUX: {}", ddi.name, err); - match gmbus_read_edid(ddi) { - Ok(edid_data) => ("GMBUS", edid_data), - Err(err) => { - log::debug!("DDI {} failed to read EDID from GMBUS: {}", ddi.name, err); - match gpio_read_edid(ddi) { - Ok(edid_data) => ("GPIO", edid_data), - Err(err) => { - log::debug!( - "DDI {} failed to read EDID from GPIO: {}", - ddi.name, - err - ); - // Will try again but not fail the driver - return Ok(false); - } - } - } - } - } + let Some((source, edid_data)) = + ddi.probe_edid(&mut self.power_wells, &self.gttmm, &mut self.gmbus)? + else { + return Ok(false); }; let edid = match edid::parse(&edid_data).to_full_result() { @@ -667,16 +573,10 @@ impl Device { } }; - let mut timing_opt = None; - for desc in edid.descriptors.iter() { - match desc { - edid::Descriptor::DetailedTiming(timing) => { - timing_opt = Some(timing); - break; - } - _ => {} - } - } + let timing_opt = edid.descriptors.iter().find_map(|desc| match desc { + edid::Descriptor::DetailedTiming(timing) => Some(timing), + _ => None, + }); let Some(timing) = timing_opt else { log::warn!( "DDI {} EDID from {} missing detailed timing", From f9fa43df40ecfac0a6f0bf0697a823563294b901 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Dec 2025 12:14:23 +0100 Subject: [PATCH 5/6] drivers/graphics/ihdgd: Move I2cBB creation to GpioPort --- drivers/graphics/ihdgd/src/device/ddi.rs | 14 +++------ drivers/graphics/ihdgd/src/device/gpio.rs | 38 ++++++++++++++++------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/drivers/graphics/ihdgd/src/device/ddi.rs b/drivers/graphics/ihdgd/src/device/ddi.rs index 67fb9cebb5..64c51bfd91 100644 --- a/drivers/graphics/ihdgd/src/device/ddi.rs +++ b/drivers/graphics/ihdgd/src/device/ddi.rs @@ -2,12 +2,11 @@ use common::io::{Io, MmioPtr, WriteOnly}; use common::timeout::Timeout; use embedded_hal::prelude::*; use std::sync::Arc; -use std::time::Duration; use syscall::error::{Error, Result, EIO}; use crate::device::aux::Aux; use crate::device::power::PowerWells; -use crate::device::{CallbackGuard, Gmbus, HalTimer}; +use crate::device::{CallbackGuard, Gmbus}; use super::{GpioPort, MmioRegion}; @@ -269,14 +268,9 @@ impl Ddi { }; let mut edid_data = [0; 128]; - let i2c_freq = 100_000.0; - bitbang_hal::i2c::I2cBB::new( - unsafe { port.clock(gttmm)? }, - unsafe { port.data(gttmm)? }, - HalTimer::new(Duration::from_secs_f64(1.0 / i2c_freq)), - ) - .write_read(0x50, &[0x00], &mut edid_data) - .map_err(|_err| Error::new(EIO))?; + unsafe { port.i2c(gttmm)? } + .write_read(0x50, &[0x00], &mut edid_data) + .map_err(|_err| Error::new(EIO))?; Ok(edid_data) }; diff --git a/drivers/graphics/ihdgd/src/device/gpio.rs b/drivers/graphics/ihdgd/src/device/gpio.rs index ab24f74b35..9999b871eb 100644 --- a/drivers/graphics/ihdgd/src/device/gpio.rs +++ b/drivers/graphics/ihdgd/src/device/gpio.rs @@ -1,8 +1,12 @@ use std::convert::Infallible; +use std::time::Duration; use common::io::{Io, MmioPtr}; +use embedded_hal::blocking::i2c; use embedded_hal::digital::v2 as digital; +use crate::device::HalTimer; + use super::MmioRegion; const GPIO_DIR_MASK: u32 = 1 << 0; @@ -35,18 +39,28 @@ pub enum GpioPort { } impl GpioPort { - pub unsafe fn clock(&self, gttmm: &MmioRegion) -> syscall::Result { - Ok(GpioPin { - ctl: gttmm.mmio(*self as usize)?, - shift: GPIO_CLOCK_SHIFT, - }) - } - - pub unsafe fn data(&self, gttmm: &MmioRegion) -> syscall::Result { - Ok(GpioPin { - ctl: gttmm.mmio(*self as usize)?, - shift: GPIO_DATA_SHIFT, - }) + pub unsafe fn i2c( + &self, + gttmm: &MmioRegion, + ) -> syscall::Result> { + let i2c_freq = 100_000.0; + let (scl, sda) = unsafe { + ( + GpioPin { + ctl: gttmm.mmio(*self as usize)?, + shift: GPIO_CLOCK_SHIFT, + }, + GpioPin { + ctl: gttmm.mmio(*self as usize)?, + shift: GPIO_DATA_SHIFT, + }, + ) + }; + Ok(bitbang_hal::i2c::I2cBB::new( + scl, + sda, + HalTimer::new(Duration::from_secs_f64(1.0 / i2c_freq)), + )) } } From d76d583048069f61f26067ae7cdf3bdf9bf12726 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Dec 2025 12:29:32 +0100 Subject: [PATCH 6/6] drivers/graphics/ihdgd: Don't fail when physmap of the BIOS region fails --- drivers/graphics/ihdgd/src/device/mod.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/graphics/ihdgd/src/device/mod.rs b/drivers/graphics/ihdgd/src/device/mod.rs index 916afda64c..88927ee094 100644 --- a/drivers/graphics/ihdgd/src/device/mod.rs +++ b/drivers/graphics/ihdgd/src/device/mod.rs @@ -269,14 +269,20 @@ impl Device { log::info!("BIOS {:X?}", bios_base); // This is the default BIOS size let bios_size = 8 * 1024; - match Bios::new(MmioRegion::new( + match MmioRegion::new( bios_base as usize, bios_size, common::MemoryType::Uncacheable, - )?) { - Ok(bios) => Some(bios), + ) { + Ok(region) => match Bios::new(region) { + Ok(bios) => Some(bios), + Err(err) => { + log::warn!("failed to parse BIOS at {:08X}: {}", bios_base, err); + None + } + }, Err(err) => { - log::warn!("failed to parse BIOS at {:08X}: {}", bios_base, err); + log::warn!("failed to map BIOS at {:08X}: {}", bios_base, err); None } }