From 4c4ab2de442366ca14d313f52e36b50e0292e989 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 22 Dec 2024 11:04:53 +0100 Subject: [PATCH] graphics: Implement handoff from vesad to virtio-gpud With this virtio-gpud no longer has to support getting deactivated at any time to hand back control to vesad. This makes it much easier to add many new features that a proper graphics driver is supposed to support. Be aware that virtio-gpud waits for the vsync on every flush request. Fbcond and orbital are not really happy about this right now and when something changes multiple times within a single frame, flush requests queue up, causing the ui to hang until all flush requests are finished. It is still possible to switch to another VT though which won't hang. --- graphics/fbcond/src/display.rs | 38 +++++++++++ graphics/fbcond/src/main.rs | 9 ++- graphics/fbcond/src/text.rs | 4 ++ graphics/vesad/src/scheme.rs | 2 +- graphics/virtio-gpud/src/main.rs | 15 ----- graphics/virtio-gpud/src/scheme.rs | 93 ++++---------------------- inputd/src/lib.rs | 5 ++ inputd/src/main.rs | 103 +++++++++++++++++++++++++++-- 8 files changed, 167 insertions(+), 102 deletions(-) diff --git a/graphics/fbcond/src/display.rs b/graphics/fbcond/src/display.rs index 39b615e5a9..ceaf48da44 100644 --- a/graphics/fbcond/src/display.rs +++ b/graphics/fbcond/src/display.rs @@ -60,6 +60,7 @@ impl Drop for DisplayMap { enum DisplayCommand { Resize { width: usize, height: usize }, + ReopenForHandoff { display_path: String }, SyncRects(Vec), } @@ -104,6 +105,29 @@ impl Display { } } } + DisplayCommand::ReopenForHandoff { display_path } => { + eprintln!("fbcond: Performing handoff for '{display_path}'"); + + let (mut new_display_file, width, height) = + Self::open_display(&display_path).unwrap(); + + eprintln!("fbcond: Opened new display '{display_path}'"); + + match display_fd_map(width, height, &mut new_display_file) { + Ok(ok) => { + *map_clone.lock().unwrap() = ok; + display_file = new_display_file; + + eprintln!("fbcond: Mapped new display '{display_path}'"); + } + Err(err) => { + eprintln!( + "failed to resize display to {}x{}: {}", + width, height, err + ); + } + } + } DisplayCommand::SyncRects(sync_rects) => { for sync_rect in sync_rects { unsafe { @@ -130,6 +154,20 @@ impl Display { }) } + /// Re-open the display after a handoff. + /// + /// Once re-opening is finished, you must call [`resize`] to map the new framebuffer. + /// + /// Warning: This must be called in a background thread to avoid a deadlock when the + /// graphics driver (indirectly) writes logs to fbcond. + pub fn reopen_for_handoff(&mut self) { + let display_path = Self::display_path(&mut self.input_handle).unwrap(); + + self.cmd_tx + .send(DisplayCommand::ReopenForHandoff { display_path }) + .unwrap(); + } + fn display_path(input_handle: &mut File) -> io::Result { let mut buffer = [0; 1024]; let fd = input_handle.as_raw_fd(); diff --git a/graphics/fbcond/src/main.rs b/graphics/fbcond/src/main.rs index 7449d5a4d7..f5d5b8d243 100644 --- a/graphics/fbcond/src/main.rs +++ b/graphics/fbcond/src/main.rs @@ -1,3 +1,5 @@ +#![feature(io_error_more)] + use event::EventQueue; use orbclient::Event; use std::fs::{File, OpenOptions}; @@ -55,7 +57,9 @@ fn inner(daemon: redox_daemon::Daemon, vt_ids: &[usize]) -> ! { let mut inputd_control_handle = inputd::ControlHandle::new().unwrap(); - libredox::call::setrens(0, 0).expect("fbcond: failed to enter null namespace"); + // This is not possible for now as fbcond needs to open new displays at runtime for graphics + // driver handoff. In the future inputd may directly pass a handle to the display instead. + //libredox::call::setrens(0, 0).expect("fbcond: failed to enter null namespace"); daemon.ready().expect("failed to notify parent"); @@ -127,6 +131,9 @@ fn handle_event( Err(err) if err.kind() == ErrorKind::WouldBlock => { break; } + Err(err) if err.kind() == ErrorKind::StaleNetworkFileHandle => { + vt.handle_handoff(); + } Ok(count) => { let events = &mut events[..count]; diff --git a/graphics/fbcond/src/text.rs b/graphics/fbcond/src/text.rs index 46afcc738c..d62b15f502 100644 --- a/graphics/fbcond/src/text.rs +++ b/graphics/fbcond/src/text.rs @@ -119,6 +119,10 @@ impl TextScreen { } } + pub fn handle_handoff(&mut self) { + self.display.reopen_for_handoff(); + } + pub fn resize(&mut self, width: usize, height: usize) { self.display .resize(width.try_into().unwrap(), height.try_into().unwrap()); diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 3dd2b3a9ef..84dcb3ec39 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -34,7 +34,7 @@ pub struct DisplayScheme { impl DisplayScheme { pub fn new(framebuffers: Vec, spec: &[()]) -> DisplayScheme { - let mut inputd_handle = inputd::DisplayHandle::new("vesa").unwrap(); + let mut inputd_handle = inputd::DisplayHandle::new_early("vesa").unwrap(); let mut vts = BTreeMap::>::new(); diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index 8a40452427..ab36a1048e 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -25,14 +25,12 @@ use std::cell::UnsafeCell; use std::os::fd::AsRawFd; use std::sync::atomic::{AtomicU32, Ordering}; -use std::sync::Arc; use event::{user_data, EventQueue}; use libredox::errno::EAGAIN; use pcid_interface::PciFunctionHandle; use redox_scheme::{RequestKind, SignalBehavior, Socket, V2}; -use virtio_core::transport::{self, Queue}; use virtio_core::utils::VolatileCell; use virtio_core::MSIX_PRIMARY_VECTOR; @@ -411,19 +409,6 @@ impl XferToHost2d { static DEVICE: spin::Once = spin::Once::new(); -fn reinit(control_queue: Arc, cursor_queue: Arc) -> Result<(), transport::Error> { - let device = DEVICE.get().unwrap(); - - virtio_core::reinit(device)?; - device.transport.finalize_features(); - - device.transport.reinit_queue(control_queue.clone()); - device.transport.reinit_queue(cursor_queue.clone()); - - device.transport.run_device(); - Ok(()) -} - fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { let mut pcid_handle = PciFunctionHandle::connect_default()?; diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 0d8dc80414..97c4a0f031 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -1,7 +1,7 @@ use std::cell::RefCell; use std::collections::{BTreeMap, HashMap}; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use common::{dma::Dma, sgl}; @@ -40,8 +40,6 @@ pub struct Display<'a> { height: u32, id: usize, - - is_reseted: AtomicBool, } impl<'a> Display<'a> { @@ -64,28 +62,9 @@ impl<'a> Display<'a> { transport, id, - - is_reseted: AtomicBool::new(false), } } - async fn init(&self, vt: usize) -> Result<(), Error> { - if !self.is_reseted.load(Ordering::SeqCst) { - // The device is already initialized. - self.set_scanout(vt).await?; - return Ok(()); - } - - self.is_reseted.store(false, Ordering::SeqCst); - - log::info!("virtio-gpu: initializing GPU after a reset"); - - crate::reinit(self.control_queue.clone(), self.cursor_queue.clone())?; - self.set_scanout(vt).await?; - - Ok(()) - } - async fn get_fpath(&self, buffer: &mut [u8]) -> Result { let path = format!("display.virtio-gpu:3.0/{}/{}", self.width, self.height); @@ -199,9 +178,12 @@ impl<'a> Display<'a> { /// If `damage` is `None`, the entire screen is flushed. async fn flush(&self, vt: usize, damage: Option<&Damage>) -> Result<(), Error> { - if vt != *self.active_vt.borrow() { + let Some(&res_id) = self.vts_res.borrow().get(&vt) else { + // The resource is not yet created. Ignore the damage. We will write the entire backing + // storage to the resource once we create the resource, which is equivalent to damaging + // the entire resource. return Ok(()); - } + }; let damage = if let Some(damage) = damage { damage.into() @@ -215,7 +197,7 @@ impl<'a> Display<'a> { }; let req = Dma::new(XferToHost2d::new( - self.vts_res.borrow()[&vt], + res_id, GpuRect { x: 0, y: 0, @@ -226,31 +208,8 @@ impl<'a> Display<'a> { let header = self.send_request(req).await?; assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - self.flush_resource(ResourceFlush::new( - self.vts_res.borrow()[&vt], - damage.clone(), - )) - .await?; - Ok(()) - } - - /// This detaches any backing pages from the display and unrefs the resource. Also resets the - /// device, which is required to go back to legacy mode. - async fn detach(&self) -> Result<(), Error> { - for (_vt, res_id) in self.vts_res.borrow_mut().drain() { - let request = Dma::new(DetachBacking::new(res_id))?; - let header = self.send_request(request).await?; - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - - let request = Dma::new(ResourceUnref::new(res_id))?; - let header = self.send_request(request).await?; - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - } - - // Go back to legacy mode. - self.transport.reset(); - self.is_reseted.store(true, Ordering::SeqCst); - + self.flush_resource(ResourceFlush::new(res_id, damage.clone())) + .await?; Ok(()) } } @@ -283,9 +242,7 @@ impl<'a> Scheme<'a> { ) .await?; - let mut inputd_handle = inputd::DisplayHandle::new("virtio-gpu").unwrap(); - // FIXME make vesad handoff control over all it's VT's instead - inputd_handle.register_vt().unwrap(); + let inputd_handle = inputd::DisplayHandle::new("virtio-gpu").unwrap(); Ok(Self { handles: BTreeMap::new(), @@ -319,11 +276,6 @@ impl<'a> Scheme<'a> { transport.clone(), id, ); - // FIXME this is a hack to avoid breaking things while we need to co-exist with vesad - // Somehow necessary to ensure that creating a resource on the first reinitialization - // after this detach doesn't fail. - display.init(1).await?; - display.detach().await?; result.push(Arc::new(display)); } @@ -357,7 +309,7 @@ impl<'a> Scheme<'a> { for display in &self.displays { log::warn!("virtio-gpu: activating"); - futures::executor::block_on(display.init(vt_event.vt)).unwrap(); + futures::executor::block_on(display.set_scanout(vt_event.vt)).unwrap(); *display.active_vt.borrow_mut() = vt_event.vt; } @@ -365,21 +317,7 @@ impl<'a> Scheme<'a> { VtEventKind::Deactivate => { log::info!("deactivate {}", vt_event.vt); - - for handle in self.handles.values() { - if handle.vt != vt_event.vt { - continue; - } - - log::warn!("virtio-gpu: deactivating"); - - futures::executor::block_on(handle.display.detach()).unwrap(); - break; - } - - // for display in self.displays.iter() { - // futures::executor::block_on(display.detach()).unwrap(); - // } + // nothing to do :) } VtEventKind::Resize => { @@ -449,13 +387,6 @@ impl<'a> SchemeMut for Scheme<'a> { ) -> syscall::Result { let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; - // The VT is not active and the device is reseted. Ignore the damage. We will recreate the - // backing storage from scratch next time we initialize, which is equivalent to damaging the - // entire buffer. - if handle.display.is_reseted.load(Ordering::SeqCst) { - return Ok(buf.len()); - } - let damages = unsafe { core::slice::from_raw_parts( buf.as_ptr() as *const Damage, diff --git a/inputd/src/lib.rs b/inputd/src/lib.rs index 907ea53a9b..ca8b25750f 100644 --- a/inputd/src/lib.rs +++ b/inputd/src/lib.rs @@ -27,6 +27,11 @@ impl DisplayHandle { Ok(Self(File::open(path)?)) } + pub fn new_early>(device_name: S) -> Result { + let path = format!("/scheme/input/handle_early/display/{}", device_name.into()); + Ok(Self(File::open(path)?)) + } + // The return value is the display identifier. It will be used to uniquely // identify the display on activation events. pub fn register_vt(&mut self) -> Result { diff --git a/inputd/src/main.rs b/inputd/src/main.rs index 569e1d80e4..a79ba703df 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -18,6 +18,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use inputd::{VtActivate, VtEvent, VtEventKind}; +use libredox::errno::ESTALE; use redox_scheme::{RequestKind, SchemeMut, SignalBehavior, Socket, V2}; use orbclient::{Event, EventOption}; @@ -28,6 +29,9 @@ enum Handle { Consumer { events: EventFlags, pending: Vec, + /// We return an ESTALE error once to indicate that a handoff to a different graphics driver + /// is necessary. + needs_handoff: bool, notified: bool, vt: usize, }, @@ -36,6 +40,8 @@ enum Handle { pending: Vec, notified: bool, device: String, + /// Control of all VT's gets handed over from earlyfb devices to the first non-earlyfb device. + is_earlyfb: bool, }, Control, } @@ -46,6 +52,7 @@ impl Handle { } } +#[derive(Debug)] struct Vt { display: String, } @@ -69,6 +76,7 @@ struct InputScheme { active_vt: Option, has_new_events: bool, + maybe_perform_handoff_to: Option, } impl InputScheme { @@ -83,6 +91,7 @@ impl InputScheme { active_vt: None, has_new_events: false, + maybe_perform_handoff_to: None, } } @@ -163,17 +172,40 @@ impl SchemeMut for InputScheme { Handle::Consumer { events: EventFlags::empty(), pending: Vec::new(), + needs_handoff: false, notified: false, vt: target, } } - "handle" => { + "handle_early" => { let display = path_parts.collect::>().join("."); Handle::Display { events: EventFlags::empty(), pending: Vec::new(), notified: false, device: display, + is_earlyfb: true, + } + } + "handle" => { + let display = path_parts.collect::>().join("."); + self.maybe_perform_handoff_to = Some(display.clone()); + Handle::Display { + events: EventFlags::empty(), + pending: if let Some(active_vt) = self.active_vt { + vec![VtEvent { + kind: VtEventKind::Activate, + vt: active_vt, + width: 0, + height: 0, + stride: 0, + }] + } else { + vec![] + }, + notified: false, + device: display, + is_earlyfb: false, } } "control" => Handle::Control, @@ -216,7 +248,17 @@ impl SchemeMut for InputScheme { let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; match handle { - Handle::Consumer { pending, .. } => { + Handle::Consumer { + pending, + needs_handoff, + .. + } => { + if *needs_handoff { + *needs_handoff = false; + // Indicates that handoff to a new graphics driver is necessary. + return Err(SysError::new(ESTALE)); + } + let copy = core::cmp::min(pending.len(), buf.len()); for (i, byte) in pending.drain(..copy).enumerate() { @@ -464,6 +506,55 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { RequestKind::MsyncMsg | RequestKind::MunmapMsg | RequestKind::MmapMsg => unreachable!(), } + if let Some(display) = scheme.maybe_perform_handoff_to.take() { + let early_displays = scheme + .handles + .values() + .filter_map(|handle| match handle { + Handle::Display { + device, + is_earlyfb: true, + .. + } => Some(&**device), + _ => None, + }) + .collect::>(); + let vts = scheme + .vts + .iter_mut() + .filter_map(|(&i, vt)| { + if early_displays.contains(&&*vt.display) { + vt.display = display.clone(); + + scheme.has_new_events = true; + + Some(i) + } else { + None + } + }) + .collect::>(); + + for handle in scheme.handles.values_mut() { + match handle { + Handle::Consumer { + needs_handoff, + notified, + vt, + .. + } => { + if !vts.contains(vt) { + continue; + } + + *needs_handoff = true; + *notified = false; + } + _ => continue, + } + } + } + if !scheme.has_new_events { continue; } @@ -473,10 +564,14 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { Handle::Consumer { events, pending, + needs_handoff, ref mut notified, vt, } => { - if pending.is_empty() || *notified || !events.contains(EventFlags::EVENT_READ) { + if (!*needs_handoff && pending.is_empty()) + || *notified + || !events.contains(EventFlags::EVENT_READ) + { continue; } @@ -484,7 +579,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { // The activate VT is not the same as the VT that the consumer is listening to // for events. - if active_vt != *vt { + if !*needs_handoff && active_vt != *vt { continue; }