From 5ddddd91dfc949c96bca06d700d0b7eb980baec8 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 29 Jun 2025 18:01:40 +0200 Subject: [PATCH 1/8] graphics: Pass CursorPlane by shared ref to handle_cursor --- graphics/driver-graphics/src/lib.rs | 2 +- graphics/vesad/src/scheme.rs | 2 +- graphics/virtio-gpud/src/scheme.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 3ef5c0874e..09464cc083 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -22,7 +22,7 @@ pub trait GraphicsAdapter { fn supports_hw_cursor(&self) -> bool; fn create_cursor_framebuffer(&mut self) -> Self::Cursor; fn map_cursor_framebuffer(&mut self, cursor: &Self::Cursor) -> *mut u8; - fn handle_cursor(&mut self, cursor: &mut CursorPlane, dirty_fb: bool); + fn handle_cursor(&mut self, cursor: &CursorPlane, dirty_fb: bool); } pub trait Framebuffer { diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index f8c4249ee5..0c140e4767 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -55,7 +55,7 @@ impl GraphicsAdapter for FbAdapter { unimplemented!("Vesad does not support this function"); } - fn handle_cursor(&mut self, _cursor: &mut CursorPlane, _dirty_fb: bool) { + fn handle_cursor(&mut self, _cursor: &CursorPlane, _dirty_fb: bool) { unimplemented!("Vesad does not support this function"); } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 86fc9c9fc9..1a908fbb68 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -331,7 +331,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { cursor.sgl.as_ptr() } - fn handle_cursor(&mut self, cursor: &mut CursorPlane, dirty_fb: bool) { + fn handle_cursor(&mut self, cursor: &CursorPlane, dirty_fb: bool) { if dirty_fb { self.update_cursor( &cursor.framebuffer, From 56d4fdd025ab72c33b110ba2aedd29d08c4f65af Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 29 Jun 2025 19:48:30 +0200 Subject: [PATCH 2/8] graphics: Replace adapter.displays() with .display_count() --- graphics/driver-graphics/src/lib.rs | 6 +++--- graphics/vesad/src/scheme.rs | 4 ++-- graphics/virtio-gpud/src/scheme.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 09464cc083..9d6bc87097 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -11,7 +11,7 @@ pub trait GraphicsAdapter { type Framebuffer: Framebuffer; type Cursor: CursorFramebuffer; - fn displays(&self) -> Vec; + fn display_count(&self) -> usize; fn display_size(&self, display_id: usize) -> (u32, u32); fn create_dumb_framebuffer(&mut self, width: u32, height: u32) -> Self::Framebuffer; @@ -91,7 +91,7 @@ impl GraphicsScheme { VtEventKind::Activate => { log::info!("activate {}", vt_event.vt); - for display_id in self.adapter.displays() { + for display_id in 0..self.adapter.display_count() { let framebuffer = self .vts_fb .entry(vt_event.vt) @@ -195,7 +195,7 @@ impl Scheme for GraphicsScheme { let vt = screen.next().unwrap_or("").parse::().unwrap(); let id = screen.next().unwrap_or("").parse::().unwrap_or(0); - if id >= self.adapter.displays().len() { + if id >= self.adapter.display_count() { return Err(Error::new(EINVAL)); } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 0c140e4767..63a83896c8 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -20,8 +20,8 @@ impl GraphicsAdapter for FbAdapter { type Framebuffer = GraphicScreen; type Cursor = VesadCursor; - fn displays(&self) -> Vec { - (0..self.framebuffers.len()).collect() + fn display_count(&self) -> usize { + self.framebuffers.len() } fn display_size(&self, display_id: usize) -> (u32, u32) { diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 1a908fbb68..0bb3ee52e9 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -149,8 +149,8 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { type Framebuffer = VirtGpuFramebuffer; type Cursor = VirtGpuCursor; - fn displays(&self) -> Vec { - self.displays.iter().enumerate().map(|(i, _)| i).collect() + fn display_count(&self) -> usize { + self.displays.len() } fn display_size(&self, display_id: usize) -> (u32, u32) { From 74bb9f40e51963d0d44545a3ac1000f22731c24b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 29 Jun 2025 19:49:57 +0200 Subject: [PATCH 3/8] graphics/graphics-ipc: Introduce a common module for code shared between versions --- graphics/graphics-ipc/src/common.rs | 75 ++++++++++++++++++++++++++ graphics/graphics-ipc/src/lib.rs | 1 + graphics/graphics-ipc/src/v1.rs | 81 +++-------------------------- 3 files changed, 83 insertions(+), 74 deletions(-) create mode 100644 graphics/graphics-ipc/src/common.rs diff --git a/graphics/graphics-ipc/src/common.rs b/graphics/graphics-ipc/src/common.rs new file mode 100644 index 0000000000..81e02640d5 --- /dev/null +++ b/graphics/graphics-ipc/src/common.rs @@ -0,0 +1,75 @@ +use std::cmp; + +// Keep synced with orbital's SyncRect +// Technically orbital uses i32 rather than u32, but values larger than i32::MAX +// would be a bug anyway. +#[derive(Debug, Copy, Clone)] +#[repr(C, packed)] +pub struct Damage { + pub x: u32, + pub y: u32, + pub width: u32, + pub height: u32, +} + +impl Damage { + #[must_use] + pub fn clip(mut self, width: u32, height: u32) -> Self { + // Clip damage + let x2 = self.x + self.width; + self.x = cmp::min(self.x, width); + if x2 > width { + self.width = width - self.x; + } + + let y2 = self.y + self.height; + self.y = cmp::min(self.y, height); + if y2 > height { + self.height = height - self.y; + } + self + } +} + +pub struct DisplayMap { + offscreen: *mut [u32], + width: usize, + height: usize, +} + +impl DisplayMap { + pub(crate) unsafe fn new(offscreen: *mut [u32], width: usize, height: usize) -> Self { + DisplayMap { + offscreen, + width, + height, + } + } + + pub fn ptr(&self) -> *const [u32] { + self.offscreen + } + + pub fn ptr_mut(&mut self) -> *mut [u32] { + self.offscreen + } + + pub fn width(&self) -> usize { + self.width + } + + pub fn height(&self) -> usize { + self.height + } +} + +unsafe impl Send for DisplayMap {} +unsafe impl Sync for DisplayMap {} + +impl Drop for DisplayMap { + fn drop(&mut self) { + unsafe { + let _ = libredox::call::munmap(self.offscreen as *mut (), self.offscreen.len()); + } + } +} diff --git a/graphics/graphics-ipc/src/lib.rs b/graphics/graphics-ipc/src/lib.rs index a3a6d96c3f..99b52a835b 100644 --- a/graphics/graphics-ipc/src/lib.rs +++ b/graphics/graphics-ipc/src/lib.rs @@ -1 +1,2 @@ +mod common; pub mod v1; diff --git a/graphics/graphics-ipc/src/v1.rs b/graphics/graphics-ipc/src/v1.rs index 14fb1e10b4..a29258db9c 100644 --- a/graphics/graphics-ipc/src/v1.rs +++ b/graphics/graphics-ipc/src/v1.rs @@ -1,13 +1,16 @@ use std::fs::File; use std::os::unix::io::AsRawFd; -use std::{cmp, io, mem, ptr, slice}; +use std::{io, mem, ptr, slice}; use libredox::flag; +pub use crate::common::Damage; +pub use crate::common::DisplayMap; + /// A graphics handle using the v1 graphics API. /// -/// The v1 graphics API only allows a single framebuffer for each VT and supports neither page -/// flipping nor cursor planes. +/// The v1 graphics API only allows a single framebuffer for each VT, requires each display to be +/// handled separately and doesn't support page flipping. pub struct V1GraphicsHandle { file: File, } @@ -52,11 +55,7 @@ impl V1GraphicsHandle { }; let offscreen = ptr::slice_from_raw_parts_mut(display_ptr as *mut u32, width * height); - Ok(DisplayMap { - offscreen, - width, - height, - }) + Ok(unsafe { DisplayMap::new(offscreen, width, height) }) } pub fn sync_full_screen(&self) -> io::Result<()> { @@ -75,41 +74,6 @@ impl V1GraphicsHandle { } } -pub struct DisplayMap { - offscreen: *mut [u32], - width: usize, - height: usize, -} - -impl DisplayMap { - pub fn ptr(&self) -> *const [u32] { - self.offscreen - } - - pub fn ptr_mut(&mut self) -> *mut [u32] { - self.offscreen - } - - pub fn width(&self) -> usize { - self.width - } - - pub fn height(&self) -> usize { - self.height - } -} - -unsafe impl Send for DisplayMap {} -unsafe impl Sync for DisplayMap {} - -impl Drop for DisplayMap { - fn drop(&mut self) { - unsafe { - let _ = libredox::call::munmap(self.offscreen as *mut (), self.offscreen.len()); - } - } -} - #[derive(Debug, Copy, Clone)] #[repr(C, packed)] pub struct CursorDamage { @@ -122,34 +86,3 @@ pub struct CursorDamage { pub height: i32, pub cursor_img_bytes: [u32; 4096], } - -// Keep synced with orbital's SyncRect -// Technically orbital uses i32 rather than u32, but values larger than i32::MAX -// would be a bug anyway. -#[derive(Debug, Copy, Clone)] -#[repr(packed)] -pub struct Damage { - pub x: u32, - pub y: u32, - pub width: u32, - pub height: u32, -} - -impl Damage { - #[must_use] - pub fn clip(mut self, width: u32, height: u32) -> Self { - // Clip damage - let x2 = self.x + self.width; - self.x = cmp::min(self.x, width); - if x2 > width { - self.width = width - self.x; - } - - let y2 = self.y + self.height; - self.y = cmp::min(self.y, height); - if y2 > height { - self.height = height - self.y; - } - self - } -} From 993b23e63c3f1d3d96aa9a7c20ee0c24b95f1064 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 30 Jun 2025 20:04:40 +0200 Subject: [PATCH 4/8] graphics/driver-graphics: Prepare for future abi versions --- graphics/driver-graphics/src/lib.rs | 190 +++++++++++++++------------- 1 file changed, 100 insertions(+), 90 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 9d6bc87097..b4f78525d1 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -54,7 +54,7 @@ pub struct GraphicsScheme { } enum Handle { - Screen { vt: usize, screen: usize }, + V1Screen { vt: usize, screen: usize }, } impl GraphicsScheme { @@ -210,33 +210,39 @@ impl Scheme for GraphicsScheme { self.next_id += 1; self.handles - .insert(self.next_id, Handle::Screen { vt, screen: id }); + .insert(self.next_id, Handle::V1Screen { vt, screen: id }); Ok(self.next_id) } fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { - let Handle::Screen { vt, screen } = self.handles.get(&id).ok_or(Error::new(EBADF))?; - let framebuffer = &self.vts_fb[vt][screen]; - let path = format!( - "{}:{vt}.{screen}/{}/{}", - self.scheme_name, - framebuffer.width(), - framebuffer.height() - ); + let path = match self.handles.get(&id).ok_or(Error::new(EBADF))? { + Handle::V1Screen { vt, screen } => { + let framebuffer = &self.vts_fb[vt][screen]; + format!( + "{}:{vt}.{screen}/{}/{}", + self.scheme_name, + framebuffer.width(), + framebuffer.height() + ) + } + }; buf[..path.len()].copy_from_slice(path.as_bytes()); Ok(path.len()) } fn fsync(&mut self, id: usize) -> syscall::Result { - let Handle::Screen { vt, screen } = self.handles.get(&id).ok_or(Error::new(EBADF))?; - if *vt != self.active_vt { - // This is a protection against background VT's spamming us with flush requests. We will - // flush the framebuffer on the next VT switch anyway - return Ok(0); + match self.handles.get(&id).ok_or(Error::new(EBADF))? { + Handle::V1Screen { vt, screen } => { + if *vt != self.active_vt { + // This is a protection against background VT's spamming us with flush requests. We will + // flush the framebuffer on the next VT switch anyway + return Ok(0); + } + let framebuffer = &self.vts_fb[vt][screen]; + Self::update_whole_screen(&mut self.adapter, *screen, framebuffer); + Ok(0) + } } - let framebuffer = &self.vts_fb[vt][screen]; - Self::update_whole_screen(&mut self.adapter, *screen, framebuffer); - Ok(0) } fn read( @@ -246,88 +252,92 @@ impl Scheme for GraphicsScheme { _offset: u64, _fcntl_flags: u32, ) -> Result { - let _handle = self.handles.get(&id).ok_or(Error::new(EBADF))?; + match self.handles.get(&id).ok_or(Error::new(EBADF))? { + Handle::V1Screen { .. } => { + //Currently read is only used for Orbital to check GPU cursor support + //and only expects a buf to pass a 0 or 1 flag + if self.adapter.supports_hw_cursor() { + buf[0] = 1; + } else { + buf[0] = 0; + } - //Currently read is only used for Orbital to check GPU cursor support - //and only expects a buf to pass a 0 or 1 flag - if self.adapter.supports_hw_cursor() { - buf[0] = 1; - } else { - buf[0] = 0; + Ok(1) + } } - - Ok(1) } fn write(&mut self, id: usize, buf: &[u8], _offset: u64, _fcntl_flags: u32) -> Result { - let Handle::Screen { vt, screen } = self.handles.get(&id).ok_or(Error::new(EBADF))?; - - if *vt != self.active_vt { - // This is a protection against background VT's spamming us with flush requests. We will - // flush the framebuffer on the next VT switch anyway - return Ok(buf.len()); - } - - if size_of_val(buf) == std::mem::size_of::() - && self.adapter.supports_hw_cursor() - { - let cursor_damage = unsafe { *buf.as_ptr().cast::() }; - - let cursor_plane = Self::cursor_plane_for_vt( - &mut self.adapter, - &mut self.cursor_planes, - self.active_vt, - ); - - cursor_plane.x = cursor_damage.x; - cursor_plane.y = cursor_damage.y; - - if cursor_damage.header == 0 { - self.adapter.handle_cursor(cursor_plane, false); - } else { - cursor_plane.hot_x = cursor_damage.hot_x; - cursor_plane.hot_y = cursor_damage.hot_y; - - let w: i32 = cursor_damage.width; - let h: i32 = cursor_damage.height; - let cursor_image = cursor_damage.cursor_img_bytes; - let cursor_ptr = self - .adapter - .map_cursor_framebuffer(&cursor_plane.framebuffer); - - //Clear previous image from backing storage - unsafe { - core::ptr::write_bytes(cursor_ptr as *mut u8, 0, 64 * 64 * 4); + match self.handles.get(&id).ok_or(Error::new(EBADF))? { + Handle::V1Screen { vt, screen } => { + if *vt != self.active_vt { + // This is a protection against background VT's spamming us with flush requests. We will + // flush the framebuffer on the next VT switch anyway + return Ok(buf.len()); } - //Write image to backing storage - for row in 0..h { - let start: usize = (w * row) as usize; - let end: usize = (w * row + w) as usize; + if size_of_val(buf) == std::mem::size_of::() + && self.adapter.supports_hw_cursor() + { + let cursor_damage = unsafe { *buf.as_ptr().cast::() }; - unsafe { - core::ptr::copy_nonoverlapping( - cursor_image[start..end].as_ptr(), - cursor_ptr.cast::().offset(64 * row as isize), - w as usize, - ); + let cursor_plane = Self::cursor_plane_for_vt( + &mut self.adapter, + &mut self.cursor_planes, + self.active_vt, + ); + + cursor_plane.x = cursor_damage.x; + cursor_plane.y = cursor_damage.y; + + if cursor_damage.header == 0 { + self.adapter.handle_cursor(cursor_plane, false); + } else { + cursor_plane.hot_x = cursor_damage.hot_x; + cursor_plane.hot_y = cursor_damage.hot_y; + + let w: i32 = cursor_damage.width; + let h: i32 = cursor_damage.height; + let cursor_image = cursor_damage.cursor_img_bytes; + let cursor_ptr = self + .adapter + .map_cursor_framebuffer(&cursor_plane.framebuffer); + + //Clear previous image from backing storage + unsafe { + core::ptr::write_bytes(cursor_ptr as *mut u8, 0, 64 * 64 * 4); + } + + //Write image to backing storage + for row in 0..h { + let start: usize = (w * row) as usize; + let end: usize = (w * row + w) as usize; + + unsafe { + core::ptr::copy_nonoverlapping( + cursor_image[start..end].as_ptr(), + cursor_ptr.cast::().offset(64 * row as isize), + w as usize, + ); + } + } + + self.adapter.handle_cursor(cursor_plane, true); } + + return Ok(buf.len()); } - self.adapter.handle_cursor(cursor_plane, true); + let framebuffer = &self.vts_fb[vt][screen]; + + assert_eq!(buf.len(), std::mem::size_of::()); + let damage = unsafe { *buf.as_ptr().cast::() }; + + self.adapter.update_plane(*screen, framebuffer, damage); + + Ok(buf.len()) } - - return Ok(buf.len()); } - - let framebuffer = &self.vts_fb[vt][screen]; - - assert_eq!(buf.len(), std::mem::size_of::()); - let damage = unsafe { *buf.as_ptr().cast::() }; - - self.adapter.update_plane(*screen, framebuffer, damage); - - Ok(buf.len()) } fn mmap_prep( @@ -338,9 +348,9 @@ impl Scheme for GraphicsScheme { _flags: MapFlags, ) -> syscall::Result { // log::trace!("KSMSG MMAP {} {:?} {} {}", id, _flags, _offset, _size); - let handle = self.handles.get(&id).ok_or(Error::new(EINVAL))?; - let Handle::Screen { vt, screen } = handle; - let framebuffer = &self.vts_fb[vt][screen]; + let framebuffer = match self.handles.get(&id).ok_or(Error::new(EINVAL))? { + Handle::V1Screen { vt, screen } => &self.vts_fb[vt][screen], + }; let ptr = T::map_dumb_framebuffer(&mut self.adapter, framebuffer); Ok(ptr as usize) } From c2249f2fa061fe15aa08e3f3adda215c36a3091e Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 30 Jun 2025 21:40:02 +0200 Subject: [PATCH 5/8] graphics/driver-graphics: Couple of minor changes --- graphics/driver-graphics/src/lib.rs | 43 +++++++++++++++++------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index b4f78525d1..3f79f6dbbd 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -1,5 +1,6 @@ use std::collections::{BTreeMap, HashMap}; use std::io; +use std::sync::Arc; use graphics_ipc::v1::{CursorDamage, Damage}; use inputd::{VtEvent, VtEventKind}; @@ -49,7 +50,7 @@ pub struct GraphicsScheme { handles: BTreeMap, active_vt: usize, - vts_fb: HashMap>, + vts_fb: HashMap>>, cursor_planes: HashMap>, } @@ -92,15 +93,12 @@ impl GraphicsScheme { log::info!("activate {}", vt_event.vt); for display_id in 0..self.adapter.display_count() { - let framebuffer = self - .vts_fb - .entry(vt_event.vt) - .or_default() - .entry(display_id) - .or_insert_with(|| { - let (width, height) = self.adapter.display_size(display_id); - self.adapter.create_dumb_framebuffer(width, height) - }); + let framebuffer = Self::framebuffer_for_vt_and_display( + &mut self.adapter, + &mut self.vts_fb, + vt_event.vt, + display_id, + ); Self::update_whole_screen(&mut self.adapter, display_id, framebuffer); self.active_vt = vt_event.vt; @@ -168,6 +166,22 @@ impl GraphicsScheme { ); } + fn framebuffer_for_vt_and_display<'a>( + adapter: &mut T, + vts_fb: &'a mut HashMap>>, + vt: usize, + display_id: usize, + ) -> &'a T::Framebuffer { + vts_fb + .entry(vt) + .or_default() + .entry(display_id) + .or_insert_with(|| { + let (width, height) = adapter.display_size(display_id); + Arc::new(adapter.create_dumb_framebuffer(width, height)) + }) + } + fn cursor_plane_for_vt<'a>( adapter: &mut T, cursor_planes: &'a mut HashMap>, @@ -199,14 +213,7 @@ impl Scheme for GraphicsScheme { return Err(Error::new(EINVAL)); } - self.vts_fb - .entry(vt) - .or_default() - .entry(id) - .or_insert_with(|| { - let (width, height) = self.adapter.display_size(id); - self.adapter.create_dumb_framebuffer(width, height) - }); + Self::framebuffer_for_vt_and_display(&mut self.adapter, &mut self.vts_fb, vt, id); self.next_id += 1; self.handles From 7882372dab47f16ae888dc92d18a1d6bd5111504 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 30 Jun 2025 22:10:02 +0200 Subject: [PATCH 6/8] graphics/driver-graphics: Introduce VtState This contains the current state for a single VT. --- graphics/driver-graphics/src/lib.rs | 66 +++++++++++++++++------------ 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 3f79f6dbbd..dd1c873fbc 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -50,8 +50,21 @@ pub struct GraphicsScheme { handles: BTreeMap, active_vt: usize, - vts_fb: HashMap>>, - cursor_planes: HashMap>, + vts: HashMap>, +} + +struct VtState { + display_fbs: HashMap>, + cursor_plane: Option>, +} + +impl Default for VtState { + fn default() -> Self { + VtState { + display_fbs: HashMap::default(), + cursor_plane: None, + } + } } enum Handle { @@ -70,8 +83,7 @@ impl GraphicsScheme { next_id: 0, handles: BTreeMap::new(), active_vt: 0, - vts_fb: HashMap::new(), - cursor_planes: HashMap::new(), + vts: HashMap::new(), } } @@ -95,7 +107,7 @@ impl GraphicsScheme { for display_id in 0..self.adapter.display_count() { let framebuffer = Self::framebuffer_for_vt_and_display( &mut self.adapter, - &mut self.vts_fb, + &mut self.vts, vt_event.vt, display_id, ); @@ -106,7 +118,7 @@ impl GraphicsScheme { if self.adapter.supports_hw_cursor() { let cursor_plane = Self::cursor_plane_for_vt( &mut self.adapter, - &mut self.cursor_planes, + &mut self.vts, self.active_vt, ); self.adapter.handle_cursor(cursor_plane, true); @@ -168,13 +180,13 @@ impl GraphicsScheme { fn framebuffer_for_vt_and_display<'a>( adapter: &mut T, - vts_fb: &'a mut HashMap>>, + vts: &'a mut HashMap>, vt: usize, display_id: usize, ) -> &'a T::Framebuffer { - vts_fb - .entry(vt) + vts.entry(vt) .or_default() + .display_fbs .entry(display_id) .or_insert_with(|| { let (width, height) = adapter.display_size(display_id); @@ -184,16 +196,19 @@ impl GraphicsScheme { fn cursor_plane_for_vt<'a>( adapter: &mut T, - cursor_planes: &'a mut HashMap>, + vts: &'a mut HashMap>, vt: usize, ) -> &'a mut CursorPlane { - cursor_planes.entry(vt).or_insert_with(|| CursorPlane { - x: 0, - y: 0, - hot_x: 0, - hot_y: 0, - framebuffer: adapter.create_cursor_framebuffer(), - }) + vts.entry(vt) + .or_default() + .cursor_plane + .get_or_insert_with(|| CursorPlane { + x: 0, + y: 0, + hot_x: 0, + hot_y: 0, + framebuffer: adapter.create_cursor_framebuffer(), + }) } } @@ -213,7 +228,7 @@ impl Scheme for GraphicsScheme { return Err(Error::new(EINVAL)); } - Self::framebuffer_for_vt_and_display(&mut self.adapter, &mut self.vts_fb, vt, id); + Self::framebuffer_for_vt_and_display(&mut self.adapter, &mut self.vts, vt, id); self.next_id += 1; self.handles @@ -224,7 +239,7 @@ impl Scheme for GraphicsScheme { fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { let path = match self.handles.get(&id).ok_or(Error::new(EBADF))? { Handle::V1Screen { vt, screen } => { - let framebuffer = &self.vts_fb[vt][screen]; + let framebuffer = &self.vts[vt].display_fbs[screen]; format!( "{}:{vt}.{screen}/{}/{}", self.scheme_name, @@ -245,7 +260,7 @@ impl Scheme for GraphicsScheme { // flush the framebuffer on the next VT switch anyway return Ok(0); } - let framebuffer = &self.vts_fb[vt][screen]; + let framebuffer = &self.vts[vt].display_fbs[screen]; Self::update_whole_screen(&mut self.adapter, *screen, framebuffer); Ok(0) } @@ -288,11 +303,8 @@ impl Scheme for GraphicsScheme { { let cursor_damage = unsafe { *buf.as_ptr().cast::() }; - let cursor_plane = Self::cursor_plane_for_vt( - &mut self.adapter, - &mut self.cursor_planes, - self.active_vt, - ); + let cursor_plane = + Self::cursor_plane_for_vt(&mut self.adapter, &mut self.vts, self.active_vt); cursor_plane.x = cursor_damage.x; cursor_plane.y = cursor_damage.y; @@ -335,7 +347,7 @@ impl Scheme for GraphicsScheme { return Ok(buf.len()); } - let framebuffer = &self.vts_fb[vt][screen]; + let framebuffer = &self.vts[vt].display_fbs[screen]; assert_eq!(buf.len(), std::mem::size_of::()); let damage = unsafe { *buf.as_ptr().cast::() }; @@ -356,7 +368,7 @@ impl Scheme for GraphicsScheme { ) -> syscall::Result { // log::trace!("KSMSG MMAP {} {:?} {} {}", id, _flags, _offset, _size); let framebuffer = match self.handles.get(&id).ok_or(Error::new(EINVAL))? { - Handle::V1Screen { vt, screen } => &self.vts_fb[vt][screen], + Handle::V1Screen { vt, screen } => &self.vts[vt].display_fbs[screen], }; let ptr = T::map_dumb_framebuffer(&mut self.adapter, framebuffer); Ok(ptr as usize) From 7f5c58892cd3b3f694c9e1575c865a3bc78c512b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 30 Jun 2025 23:01:52 +0200 Subject: [PATCH 7/8] graphics/driver-graphics: Eagerly create contents of VtState --- graphics/driver-graphics/src/lib.rs | 111 ++++++++++++---------------- 1 file changed, 48 insertions(+), 63 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index dd1c873fbc..a466c19139 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -12,6 +12,9 @@ pub trait GraphicsAdapter { type Framebuffer: Framebuffer; type Cursor: CursorFramebuffer; + /// The maximum amount of displays that could be attached. + /// + /// This must be constant for the lifetime of the graphics adapter. fn display_count(&self) -> usize; fn display_size(&self, display_id: usize) -> (u32, u32); @@ -54,19 +57,10 @@ pub struct GraphicsScheme { } struct VtState { - display_fbs: HashMap>, + display_fbs: Vec>, cursor_plane: Option>, } -impl Default for VtState { - fn default() -> Self { - VtState { - display_fbs: HashMap::default(), - cursor_plane: None, - } - } -} - enum Handle { V1Screen { vt: usize, screen: usize }, } @@ -104,26 +98,18 @@ impl GraphicsScheme { VtEventKind::Activate => { log::info!("activate {}", vt_event.vt); - for display_id in 0..self.adapter.display_count() { - let framebuffer = Self::framebuffer_for_vt_and_display( - &mut self.adapter, - &mut self.vts, - vt_event.vt, - display_id, - ); - Self::update_whole_screen(&mut self.adapter, display_id, framebuffer); + let vt_state = + Self::get_or_create_vt(&mut self.adapter, &mut self.vts, vt_event.vt); - self.active_vt = vt_event.vt; - - if self.adapter.supports_hw_cursor() { - let cursor_plane = Self::cursor_plane_for_vt( - &mut self.adapter, - &mut self.vts, - self.active_vt, - ); - self.adapter.handle_cursor(cursor_plane, true); - } + for (display_id, fb) in vt_state.display_fbs.iter().enumerate() { + Self::update_whole_screen(&mut self.adapter, display_id, fb); } + + if let Some(cursor_plane) = &vt_state.cursor_plane { + self.adapter.handle_cursor(cursor_plane, true); + } + + self.active_vt = vt_event.vt; } VtEventKind::Resize => { @@ -178,37 +164,31 @@ impl GraphicsScheme { ); } - fn framebuffer_for_vt_and_display<'a>( + fn get_or_create_vt<'a>( adapter: &mut T, vts: &'a mut HashMap>, vt: usize, - display_id: usize, - ) -> &'a T::Framebuffer { - vts.entry(vt) - .or_default() - .display_fbs - .entry(display_id) - .or_insert_with(|| { + ) -> &'a mut VtState { + vts.entry(vt).or_insert_with(|| { + let mut display_fbs = vec![]; + for display_id in 0..adapter.display_count() { let (width, height) = adapter.display_size(display_id); - Arc::new(adapter.create_dumb_framebuffer(width, height)) - }) - } + display_fbs.push(Arc::new(adapter.create_dumb_framebuffer(width, height))); + } - fn cursor_plane_for_vt<'a>( - adapter: &mut T, - vts: &'a mut HashMap>, - vt: usize, - ) -> &'a mut CursorPlane { - vts.entry(vt) - .or_default() - .cursor_plane - .get_or_insert_with(|| CursorPlane { + let cursor_plane = adapter.supports_hw_cursor().then(|| CursorPlane { x: 0, y: 0, hot_x: 0, hot_y: 0, framebuffer: adapter.create_cursor_framebuffer(), - }) + }); + + VtState { + display_fbs, + cursor_plane, + } + }) } } @@ -228,7 +208,8 @@ impl Scheme for GraphicsScheme { return Err(Error::new(EINVAL)); } - Self::framebuffer_for_vt_and_display(&mut self.adapter, &mut self.vts, vt, id); + // Ensure the VT exists such that the rest of the methods can freely access it. + Self::get_or_create_vt(&mut self.adapter, &mut self.vts, vt); self.next_id += 1; self.handles @@ -239,7 +220,7 @@ impl Scheme for GraphicsScheme { fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { let path = match self.handles.get(&id).ok_or(Error::new(EBADF))? { Handle::V1Screen { vt, screen } => { - let framebuffer = &self.vts[vt].display_fbs[screen]; + let framebuffer = &self.vts[vt].display_fbs[*screen]; format!( "{}:{vt}.{screen}/{}/{}", self.scheme_name, @@ -260,8 +241,11 @@ impl Scheme for GraphicsScheme { // flush the framebuffer on the next VT switch anyway return Ok(0); } - let framebuffer = &self.vts[vt].display_fbs[screen]; - Self::update_whole_screen(&mut self.adapter, *screen, framebuffer); + Self::update_whole_screen( + &mut self.adapter, + *screen, + &self.vts[vt].display_fbs[*screen], + ); Ok(0) } } @@ -298,13 +282,15 @@ impl Scheme for GraphicsScheme { return Ok(buf.len()); } - if size_of_val(buf) == std::mem::size_of::() - && self.adapter.supports_hw_cursor() - { - let cursor_damage = unsafe { *buf.as_ptr().cast::() }; + let vt_state = self.vts.get_mut(vt).unwrap(); - let cursor_plane = - Self::cursor_plane_for_vt(&mut self.adapter, &mut self.vts, self.active_vt); + if size_of_val(buf) == std::mem::size_of::() { + let Some(cursor_plane) = &mut vt_state.cursor_plane else { + // Hardware cursor not supported + return Err(Error::new(EINVAL)); + }; + + let cursor_damage = unsafe { *buf.as_ptr().cast::() }; cursor_plane.x = cursor_damage.x; cursor_plane.y = cursor_damage.y; @@ -347,12 +333,11 @@ impl Scheme for GraphicsScheme { return Ok(buf.len()); } - let framebuffer = &self.vts[vt].display_fbs[screen]; - assert_eq!(buf.len(), std::mem::size_of::()); let damage = unsafe { *buf.as_ptr().cast::() }; - self.adapter.update_plane(*screen, framebuffer, damage); + self.adapter + .update_plane(*screen, &vt_state.display_fbs[*screen], damage); Ok(buf.len()) } @@ -368,7 +353,7 @@ impl Scheme for GraphicsScheme { ) -> syscall::Result { // log::trace!("KSMSG MMAP {} {:?} {} {}", id, _flags, _offset, _size); let framebuffer = match self.handles.get(&id).ok_or(Error::new(EINVAL))? { - Handle::V1Screen { vt, screen } => &self.vts[vt].display_fbs[screen], + Handle::V1Screen { vt, screen } => &self.vts[vt].display_fbs[*screen], }; let ptr = T::map_dumb_framebuffer(&mut self.adapter, framebuffer); Ok(ptr as usize) From 7f3f1817751303677271e402dae77636b99a321c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 29 Jun 2025 18:06:22 +0200 Subject: [PATCH 8/8] graphics/virtio-gpud: Create DisplayHandle after registering the scheme This fixes a race-condition. It is unlikely to cause issues in practice though. --- graphics/virtio-gpud/src/scheme.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 0bb3ee52e9..b7707ab5f1 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -390,11 +390,8 @@ impl<'a> GpuScheme { } } - let inputd_handle = DisplayHandle::new("virtio-gpu").unwrap(); - - Ok(( - GraphicsScheme::new(adapter, "display.virtio-gpu".to_owned()), - inputd_handle, - )) + let scheme = GraphicsScheme::new(adapter, "display.virtio-gpu".to_owned()); + let handle = DisplayHandle::new("virtio-gpu").unwrap(); + Ok((scheme, handle)) } }