From 88cfa76e2c7ffddbbe7d8173893a3529ff065570 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 18:53:56 +0100 Subject: [PATCH 1/6] graphics: Merge set_scanout and flush_resource into update_plane The old api was pretty virtio-gpu specific. The new api is closed to how atomic mode setting on Linux works. --- graphics/driver-graphics/src/lib.rs | 9 ++++---- graphics/vesad/src/scheme.rs | 6 +---- graphics/virtio-gpud/src/main.rs | 2 +- graphics/virtio-gpud/src/scheme.rs | 35 +++++++++++++++-------------- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index c35b88bcd8..3a9b503f54 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -17,8 +17,7 @@ pub trait GraphicsAdapter { fn create_resource(&mut self, width: u32, height: u32) -> Self::Resource; fn map_resource(&mut self, resource: &Self::Resource) -> *mut u8; - fn set_scanout(&mut self, display_id: usize, resource: &Self::Resource); - fn flush_resource( + fn update_plane( &mut self, display_id: usize, resource: &Self::Resource, @@ -90,7 +89,7 @@ impl GraphicsScheme { let (width, height) = self.adapter.display_size(display_id); self.adapter.create_resource(width, height) }); - self.adapter.set_scanout(display_id, resource); + self.adapter.update_plane(display_id, resource, None); self.active_vt = vt_event.vt; } @@ -196,7 +195,7 @@ impl Scheme for GraphicsScheme { return Ok(0); } let resource = &self.vts_res[vt][screen]; - self.adapter.flush_resource(*screen, resource, None); + self.adapter.update_plane(*screen, resource, None); Ok(0) } @@ -229,7 +228,7 @@ impl Scheme for GraphicsScheme { ) }; - self.adapter.flush_resource(*screen, resource, Some(damage)); + self.adapter.update_plane(*screen, resource, Some(damage)); Ok(buf.len()) } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 48c3b0940a..47fde67e88 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -34,11 +34,7 @@ impl GraphicsAdapter for FbAdapter { resource.ptr.as_ptr().cast::() } - fn set_scanout(&mut self, display_id: usize, resource: &Self::Resource) { - self.flush_resource(display_id, resource, None); - } - - fn flush_resource( + fn update_plane( &mut self, display_id: usize, resource: &Self::Resource, diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index 6b0b62f3fe..2313808470 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -192,7 +192,7 @@ impl Default for GetDisplayInfo { static RESOURCE_ALLOC: AtomicU32 = AtomicU32::new(1); // XXX: 0 is reserved for whatever that takes `resource_id`. -#[derive(Debug, Copy, Clone)] +#[derive(PartialEq, Eq, Debug, Copy, Clone)] #[repr(C)] pub struct ResourceId(u32); diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index d80f45bbb6..bc0c1c4f98 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -44,6 +44,7 @@ impl Resource for VirtGpuResource { pub struct Display { width: u32, height: u32, + active_resource: Option, } pub struct VirtGpuAdapter<'a> { @@ -164,24 +165,9 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { resource.sgl.as_ptr() } - fn set_scanout(&mut self, display_id: usize, resource: &Self::Resource) { - futures::executor::block_on(async { - let scanout_request = Dma::new(SetScanout::new( - display_id as u32, - resource.id, - GpuRect::new(0, 0, resource.width, resource.height), - )) - .unwrap(); - let header = self.send_request(scanout_request).await.unwrap(); - assert_eq!(header.ty, CommandTy::RespOkNodata); - }); - - self.flush_resource(display_id, resource, None); - } - - fn flush_resource( + fn update_plane( &mut self, - _display_id: usize, + display_id: usize, resource: &Self::Resource, damage: Option<&[Damage]>, ) { @@ -200,6 +186,19 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { let header = self.send_request(req).await.unwrap(); assert_eq!(header.ty, CommandTy::RespOkNodata); + // FIXME once we support resizing we also need to check that the current and target size match + if self.displays[display_id].active_resource != Some(resource.id) { + let scanout_request = Dma::new(SetScanout::new( + display_id as u32, + resource.id, + GpuRect::new(0, 0, resource.width, resource.height), + )) + .unwrap(); + let header = self.send_request(scanout_request).await.unwrap(); + assert_eq!(header.ty, CommandTy::RespOkNodata); + self.displays[display_id].active_resource = Some(resource.id); + } + if let Some(damage) = damage { for damage in damage { self.flush_resource_inner(ResourceFlush::new( @@ -261,11 +260,13 @@ impl<'a> GpuScheme { adapter.displays.push(Display { width: 640, height: 480, + active_resource: None, }); } else { adapter.displays.push(Display { width: info.rect.width, height: info.rect.height, + active_resource: None, }); } } From 972e19e90018a86a1a3e8af895120e2f6a462926 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 19:13:16 +0100 Subject: [PATCH 2/6] graphics: Always pass damage to graphics drivers This way the drivers don't have to special case switching between VTs. --- graphics/driver-graphics/src/lib.rs | 26 +++++++++++------ graphics/vesad/src/scheme.rs | 24 ++-------------- graphics/virtio-gpud/src/scheme.rs | 44 ++++++----------------------- 3 files changed, 28 insertions(+), 66 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 3a9b503f54..9be69aa665 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -17,12 +17,7 @@ pub trait GraphicsAdapter { fn create_resource(&mut self, width: u32, height: u32) -> Self::Resource; fn map_resource(&mut self, resource: &Self::Resource) -> *mut u8; - fn update_plane( - &mut self, - display_id: usize, - resource: &Self::Resource, - damage: Option<&[Damage]>, - ); + fn update_plane(&mut self, display_id: usize, resource: &Self::Resource, damage: &[Damage]); } pub trait Resource { @@ -89,7 +84,7 @@ impl GraphicsScheme { let (width, height) = self.adapter.display_size(display_id); self.adapter.create_resource(width, height) }); - self.adapter.update_plane(display_id, resource, None); + Self::update_whole_screen(&mut self.adapter, display_id, resource); self.active_vt = vt_event.vt; } @@ -139,6 +134,19 @@ impl GraphicsScheme { Ok(()) } + + fn update_whole_screen(adapter: &mut T, screen: usize, resource: &T::Resource) { + adapter.update_plane( + screen, + resource, + &[Damage { + x: 0, + y: 0, + width: resource.width() as i32, + height: resource.height() as i32, + }], + ); + } } impl Scheme for GraphicsScheme { @@ -195,7 +203,7 @@ impl Scheme for GraphicsScheme { return Ok(0); } let resource = &self.vts_res[vt][screen]; - self.adapter.update_plane(*screen, resource, None); + Self::update_whole_screen(&mut self.adapter, *screen, resource); Ok(0) } @@ -228,7 +236,7 @@ impl Scheme for GraphicsScheme { ) }; - self.adapter.update_plane(*screen, resource, Some(damage)); + self.adapter.update_plane(*screen, resource, damage); Ok(buf.len()) } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 47fde67e88..f4e8647b98 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -34,28 +34,8 @@ impl GraphicsAdapter for FbAdapter { resource.ptr.as_ptr().cast::() } - fn update_plane( - &mut self, - display_id: usize, - resource: &Self::Resource, - damage: Option<&[Damage]>, - ) { - if let Some(damage) = damage { - resource.sync(&mut self.framebuffers[display_id], damage) - } else { - let framebuffer: &mut FrameBuffer = &mut self.framebuffers[display_id]; - let width = resource.width.try_into().unwrap(); - let height = resource.height.try_into().unwrap(); - resource.sync( - framebuffer, - &[Damage { - x: 0, - y: 0, - width, - height, - }], - ); - } + fn update_plane(&mut self, display_id: usize, resource: &Self::Resource, damage: &[Damage]) { + resource.sync(&mut self.framebuffers[display_id], damage) } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index bc0c1c4f98..95211bc9de 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -66,13 +66,6 @@ impl VirtGpuAdapter<'_> { Ok(header) } - async fn flush_resource_inner(&self, flush: ResourceFlush) -> Result<(), Error> { - let header = self.send_request(Dma::new(flush)?).await?; - assert_eq!(header.ty, CommandTy::RespOkNodata); - - Ok(()) - } - async fn get_display_info(&self) -> Result, Error> { let header = Dma::new(ControlHeader::with_ty(CommandTy::GetDisplayInfo))?; @@ -165,12 +158,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { resource.sgl.as_ptr() } - fn update_plane( - &mut self, - display_id: usize, - resource: &Self::Resource, - damage: Option<&[Damage]>, - ) { + fn update_plane(&mut self, display_id: usize, resource: &Self::Resource, damage: &[Damage]) { futures::executor::block_on(async { let req = Dma::new(XferToHost2d::new( resource.id, @@ -199,29 +187,15 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { self.displays[display_id].active_resource = Some(resource.id); } - if let Some(damage) = damage { - for damage in damage { - self.flush_resource_inner(ResourceFlush::new( - resource.id, - damage - .clip(resource.width as i32, resource.height as i32) - .into(), - )) - .await - .unwrap(); - } - } else { - self.flush_resource_inner(ResourceFlush::new( + for damage in damage { + let flush = ResourceFlush::new( resource.id, - GpuRect { - x: 0, - y: 0, - width: resource.width, - height: resource.height, - }, - )) - .await - .unwrap(); + damage + .clip(resource.width as i32, resource.height as i32) + .into(), + ); + let header = self.send_request(Dma::new(flush).unwrap()).await.unwrap(); + assert_eq!(header.ty, CommandTy::RespOkNodata); } }); } From fc92f0b0cef2dac1697da1da818c70a6bea65888 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 19:17:54 +0100 Subject: [PATCH 3/6] graphics: Rename the legacy graphics API to the v1 graphics API It is quite likely that the next graphics API won't be the last one and as such would become a legacy API too. Consistently using version numbers makes it easier to refer to the exact version you mean. --- graphics/console-draw/src/lib.rs | 2 +- graphics/driver-graphics/src/lib.rs | 2 +- graphics/fbbootlogd/src/display.rs | 12 ++++++------ graphics/fbcond/src/display.rs | 10 +++++----- graphics/graphics-ipc/src/lib.rs | 2 +- graphics/graphics-ipc/src/{legacy.rs => v1.rs} | 10 +++++----- graphics/vesad/src/scheme.rs | 2 +- graphics/virtio-gpud/src/scheme.rs | 2 +- 8 files changed, 21 insertions(+), 21 deletions(-) rename graphics/graphics-ipc/src/{legacy.rs => v1.rs} (93%) diff --git a/graphics/console-draw/src/lib.rs b/graphics/console-draw/src/lib.rs index bbd2d4fdeb..57820c3c09 100644 --- a/graphics/console-draw/src/lib.rs +++ b/graphics/console-draw/src/lib.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeSet, VecDeque}; use std::convert::{TryFrom, TryInto}; use std::{cmp, ptr}; -use graphics_ipc::legacy::Damage; +use graphics_ipc::v1::Damage; use orbclient::FONT; pub struct DisplayMap { diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 9be69aa665..0b276ff23f 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, HashMap}; use std::io; -use graphics_ipc::legacy::Damage; +use graphics_ipc::v1::Damage; use inputd::{VtEvent, VtEventKind}; use libredox::errno::EOPNOTSUPP; use libredox::Fd; diff --git a/graphics/fbbootlogd/src/display.rs b/graphics/fbbootlogd/src/display.rs index bd4d0c2e93..d276a24038 100644 --- a/graphics/fbbootlogd/src/display.rs +++ b/graphics/fbbootlogd/src/display.rs @@ -1,5 +1,5 @@ use event::{user_data, EventQueue}; -use graphics_ipc::legacy::{Damage, LegacyGraphicsHandle}; +use graphics_ipc::v1::{Damage, V1GraphicsHandle}; use inputd::ConsumerHandle; use libredox::errno::ESTALE; use orbclient::Event; @@ -22,7 +22,7 @@ fn read_to_slice( } } -fn display_fd_map(display_handle: LegacyGraphicsHandle) -> io::Result { +fn display_fd_map(display_handle: V1GraphicsHandle) -> io::Result { let display_map = display_handle.map_display()?; Ok(DisplayMap { display_handle: Arc::new(display_handle), @@ -31,8 +31,8 @@ fn display_fd_map(display_handle: LegacyGraphicsHandle) -> io::Result, - pub inner: graphics_ipc::legacy::DisplayMap, + display_handle: Arc, + pub inner: graphics_ipc::v1::DisplayMap, } enum DisplayCommand { @@ -50,7 +50,7 @@ impl Display { let map = match input_handle.open_display() { Ok(display) => { - let display_handle = LegacyGraphicsHandle::from_file(display)?; + let display_handle = V1GraphicsHandle::from_file(display)?; Arc::new(Mutex::new(Some( display_fd_map(display_handle) .unwrap_or_else(|e| panic!("failed to map display: {e}")), @@ -102,7 +102,7 @@ impl Display { eprintln!("fbbootlogd: handoff requested"); let new_display_handle = match input_handle.open_display() { - Ok(display) => LegacyGraphicsHandle::from_file(display).unwrap(), + Ok(display) => V1GraphicsHandle::from_file(display).unwrap(), Err(err) => { println!("fbbootlogd: No display present yet: {err}"); continue; diff --git a/graphics/fbcond/src/display.rs b/graphics/fbcond/src/display.rs index b60b8e6674..45ee86d2ca 100644 --- a/graphics/fbcond/src/display.rs +++ b/graphics/fbcond/src/display.rs @@ -1,4 +1,4 @@ -use graphics_ipc::legacy::{Damage, LegacyGraphicsHandle}; +use graphics_ipc::v1::{Damage, V1GraphicsHandle}; use inputd::ConsumerHandle; use std::io; @@ -8,8 +8,8 @@ pub struct Display { } pub struct DisplayMap { - display_handle: LegacyGraphicsHandle, - pub inner: graphics_ipc::legacy::DisplayMap, + display_handle: V1GraphicsHandle, + pub inner: graphics_ipc::v1::DisplayMap, } impl Display { @@ -63,10 +63,10 @@ impl Display { } } - fn open_display(input_handle: &ConsumerHandle) -> io::Result { + fn open_display(input_handle: &ConsumerHandle) -> io::Result { let display_file = input_handle.open_display()?; - LegacyGraphicsHandle::from_file(display_file) + V1GraphicsHandle::from_file(display_file) } pub fn sync_rects(&mut self, sync_rects: Vec) { diff --git a/graphics/graphics-ipc/src/lib.rs b/graphics/graphics-ipc/src/lib.rs index d0b036677b..a3a6d96c3f 100644 --- a/graphics/graphics-ipc/src/lib.rs +++ b/graphics/graphics-ipc/src/lib.rs @@ -1 +1 @@ -pub mod legacy; +pub mod v1; diff --git a/graphics/graphics-ipc/src/legacy.rs b/graphics/graphics-ipc/src/v1.rs similarity index 93% rename from graphics/graphics-ipc/src/legacy.rs rename to graphics/graphics-ipc/src/v1.rs index 2bfb4f2ce3..7c165f5b00 100644 --- a/graphics/graphics-ipc/src/legacy.rs +++ b/graphics/graphics-ipc/src/v1.rs @@ -4,17 +4,17 @@ use std::{cmp, io, mem, ptr, slice}; use libredox::flag; -/// A graphics handle using the legacy graphics API. +/// A graphics handle using the v1 graphics API. /// -/// The legacy graphics API only allows a single framebuffer for each VT and supports neither page +/// The v1 graphics API only allows a single framebuffer for each VT and supports neither page /// flipping nor cursor planes. -pub struct LegacyGraphicsHandle { +pub struct V1GraphicsHandle { file: File, } -impl LegacyGraphicsHandle { +impl V1GraphicsHandle { pub fn from_file(file: File) -> io::Result { - Ok(LegacyGraphicsHandle { file }) + Ok(V1GraphicsHandle { file }) } pub fn map_display(&self) -> io::Result { diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index f4e8647b98..229c3cfa8a 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -3,7 +3,7 @@ use std::convert::TryInto; use std::ptr::{self, NonNull}; use driver_graphics::{GraphicsAdapter, Resource}; -use graphics_ipc::legacy::Damage; +use graphics_ipc::v1::Damage; use syscall::PAGE_SIZE; use crate::framebuffer::FrameBuffer; diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 95211bc9de..3241294db3 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use common::{dma::Dma, sgl}; use driver_graphics::{GraphicsAdapter, GraphicsScheme, Resource}; -use graphics_ipc::legacy::Damage; +use graphics_ipc::v1::Damage; use inputd::DisplayHandle; use syscall::PAGE_SIZE; From ee3382aed0789811c4f48bc724ebb5050a7bdcd4 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 19:23:45 +0100 Subject: [PATCH 4/6] graphics/virtio-gpud: Add helper to submit a fenced request This is necessary for implementing hardware cursor support. --- graphics/virtio-gpud/src/main.rs | 3 +++ graphics/virtio-gpud/src/scheme.rs | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index 2313808470..fa2a5da6f5 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -109,6 +109,9 @@ pub enum CommandTy { static_assertions::const_assert_eq!(core::mem::size_of::(), 4); +const VIRTIO_GPU_FLAG_FENCE: u32 = 1 << 0; +//const VIRTIO_GPU_FLAG_INFO_RING_IDX: u32 = 1 << 1; + #[derive(Debug)] #[repr(C)] pub struct ControlHeader { diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 3241294db3..95714e9a77 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -66,6 +66,18 @@ impl VirtGpuAdapter<'_> { Ok(header) } + async fn send_request_fenced(&self, request: Dma) -> Result, Error> { + let mut header = Dma::new(ControlHeader::default())?; + header.flags |= VIRTIO_GPU_FLAG_FENCE; + let command = ChainBuilder::new() + .chain(Buffer::new(&request)) + .chain(Buffer::new(&header).flags(DescriptorFlags::WRITE_ONLY)) + .build(); + + self.control_queue.send(command).await; + Ok(header) + } + async fn get_display_info(&self) -> Result, Error> { let header = Dma::new(ControlHeader::with_ty(CommandTy::GetDisplayInfo))?; From 92295f2ea47475bfdca4486ca05b32e9f32e37f2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 19:41:05 +0100 Subject: [PATCH 5/6] graphics/graphics-ipc: Use u32 fields in Damage The fields should never be negative and this saves a couple of casts. --- graphics/console-draw/src/lib.rs | 2 +- graphics/driver-graphics/src/lib.rs | 4 ++-- graphics/graphics-ipc/src/v1.rs | 12 +++++++----- graphics/vesad/src/scheme.rs | 8 ++++---- graphics/virtio-gpud/src/scheme.rs | 12 +++++------- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/graphics/console-draw/src/lib.rs b/graphics/console-draw/src/lib.rs index 57820c3c09..59aea00469 100644 --- a/graphics/console-draw/src/lib.rs +++ b/graphics/console-draw/src/lib.rs @@ -221,7 +221,7 @@ impl TextScreen { } else { damage.push(Damage { x: 0, - y: i32::try_from(change).unwrap() * 16, + y: u32::try_from(change).unwrap() * 16, width, height: 16, }); diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 0b276ff23f..035ceb6a91 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -142,8 +142,8 @@ impl GraphicsScheme { &[Damage { x: 0, y: 0, - width: resource.width() as i32, - height: resource.height() as i32, + width: resource.width(), + height: resource.height(), }], ); } diff --git a/graphics/graphics-ipc/src/v1.rs b/graphics/graphics-ipc/src/v1.rs index 7c165f5b00..39226f2b31 100644 --- a/graphics/graphics-ipc/src/v1.rs +++ b/graphics/graphics-ipc/src/v1.rs @@ -111,18 +111,20 @@ impl Drop for DisplayMap { } // 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: i32, - pub y: i32, - pub width: i32, - pub height: i32, + pub x: u32, + pub y: u32, + pub width: u32, + pub height: u32, } impl Damage { #[must_use] - pub fn clip(mut self, width: i32, height: i32) -> Self { + pub fn clip(mut self, width: u32, height: u32) -> Self { // Clip damage self.x = cmp::min(self.x, width); if self.x + self.width > width { diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 229c3cfa8a..938b8a6678 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -91,10 +91,10 @@ impl GraphicScreen { self.height.try_into().unwrap(), ); - let start_x: usize = sync_rect.x.try_into().unwrap_or(0); - let start_y: usize = sync_rect.y.try_into().unwrap_or(0); - let w: usize = sync_rect.width.try_into().unwrap_or(0); - let h: usize = sync_rect.height.try_into().unwrap_or(0); + let start_x: usize = sync_rect.x.try_into().unwrap(); + let start_y: usize = sync_rect.y.try_into().unwrap(); + let w: usize = sync_rect.width.try_into().unwrap(); + let h: usize = sync_rect.height.try_into().unwrap(); let offscreen_ptr = self.ptr.as_ptr() as *mut u32; let onscreen_ptr = framebuffer.onscreen as *mut u32; // FIXME use as_mut_ptr once stable diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 95714e9a77..6bd41c91a7 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -15,10 +15,10 @@ use crate::*; impl Into for Damage { fn into(self) -> GpuRect { GpuRect { - x: self.x as u32, - y: self.y as u32, - width: self.width as u32, - height: self.height as u32, + x: self.x, + y: self.y, + width: self.width, + height: self.height, } } } @@ -202,9 +202,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { for damage in damage { let flush = ResourceFlush::new( resource.id, - damage - .clip(resource.width as i32, resource.height as i32) - .into(), + damage.clip(resource.width, resource.height).into(), ); let header = self.send_request(Dma::new(flush).unwrap()).await.unwrap(); assert_eq!(header.ty, CommandTy::RespOkNodata); From be5b8650589dc0849967db0c11eb4b4a1925977e Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 21:41:26 +0100 Subject: [PATCH 6/6] graphics: Rename resource to framebuffer This matches the Linux DRM name. --- graphics/driver-graphics/src/lib.rs | 61 ++++++++++++++++------------- graphics/vesad/src/scheme.rs | 21 ++++++---- graphics/virtio-gpud/src/scheme.rs | 41 ++++++++++--------- 3 files changed, 69 insertions(+), 54 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 035ceb6a91..d3b8fe3777 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -9,18 +9,23 @@ use redox_scheme::{RequestKind, Response, Scheme, SignalBehavior, Socket}; use syscall::{Error, MapFlags, Result, EAGAIN, EBADF, EINVAL}; pub trait GraphicsAdapter { - type Resource: Resource; + type Framebuffer: Framebuffer; fn displays(&self) -> Vec; fn display_size(&self, display_id: usize) -> (u32, u32); - fn create_resource(&mut self, width: u32, height: u32) -> Self::Resource; - fn map_resource(&mut self, resource: &Self::Resource) -> *mut u8; + fn create_dumb_framebuffer(&mut self, width: u32, height: u32) -> Self::Framebuffer; + fn map_dumb_framebuffer(&mut self, framebuffer: &Self::Framebuffer) -> *mut u8; - fn update_plane(&mut self, display_id: usize, resource: &Self::Resource, damage: &[Damage]); + fn update_plane( + &mut self, + display_id: usize, + framebuffer: &Self::Framebuffer, + damage: &[Damage], + ); } -pub trait Resource { +pub trait Framebuffer { fn width(&self) -> u32; fn height(&self) -> u32; } @@ -34,7 +39,7 @@ pub struct GraphicsScheme { handles: BTreeMap, active_vt: usize, - vts_res: HashMap>, + vts_fb: HashMap>, } enum Handle { @@ -53,7 +58,7 @@ impl GraphicsScheme { next_id: 0, handles: BTreeMap::new(), active_vt: 0, - vts_res: HashMap::new(), + vts_fb: HashMap::new(), } } @@ -75,16 +80,16 @@ impl GraphicsScheme { log::info!("activate {}", vt_event.vt); for display_id in self.adapter.displays() { - let resource = self - .vts_res + 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_resource(width, height) + self.adapter.create_dumb_framebuffer(width, height) }); - Self::update_whole_screen(&mut self.adapter, display_id, resource); + Self::update_whole_screen(&mut self.adapter, display_id, framebuffer); self.active_vt = vt_event.vt; } @@ -135,15 +140,15 @@ impl GraphicsScheme { Ok(()) } - fn update_whole_screen(adapter: &mut T, screen: usize, resource: &T::Resource) { + fn update_whole_screen(adapter: &mut T, screen: usize, framebuffer: &T::Framebuffer) { adapter.update_plane( screen, - resource, + framebuffer, &[Damage { x: 0, y: 0, - width: resource.width(), - height: resource.height(), + width: framebuffer.width(), + height: framebuffer.height(), }], ); } @@ -167,13 +172,13 @@ impl Scheme for GraphicsScheme { return Err(Error::new(EINVAL)); } - self.vts_res + self.vts_fb .entry(vt) .or_default() .entry(id) .or_insert_with(|| { let (width, height) = self.adapter.display_size(id); - self.adapter.create_resource(width, height) + self.adapter.create_dumb_framebuffer(width, height) }); self.next_id += 1; @@ -184,12 +189,12 @@ impl Scheme for GraphicsScheme { 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 resource = &self.vts_res[vt][screen]; + let framebuffer = &self.vts_fb[vt][screen]; let path = format!( "{}:{vt}.{screen}/{}/{}", self.scheme_name, - resource.width(), - resource.height() + framebuffer.width(), + framebuffer.height() ); buf[..path.len()].copy_from_slice(path.as_bytes()); Ok(path.len()) @@ -199,11 +204,11 @@ impl Scheme for GraphicsScheme { 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 resource on the next VT switch anyway + // flush the framebuffer on the next VT switch anyway return Ok(0); } - let resource = &self.vts_res[vt][screen]; - Self::update_whole_screen(&mut self.adapter, *screen, resource); + let framebuffer = &self.vts_fb[vt][screen]; + Self::update_whole_screen(&mut self.adapter, *screen, framebuffer); Ok(0) } @@ -223,11 +228,11 @@ impl Scheme for GraphicsScheme { if *vt != self.active_vt { // This is a protection against background VT's spamming us with flush requests. We will - // flush the resource on the next VT switch anyway + // flush the framebuffer on the next VT switch anyway return Ok(buf.len()); } - let resource = &self.vts_res[vt][screen]; + let framebuffer = &self.vts_fb[vt][screen]; let damage = unsafe { core::slice::from_raw_parts( @@ -236,7 +241,7 @@ impl Scheme for GraphicsScheme { ) }; - self.adapter.update_plane(*screen, resource, damage); + self.adapter.update_plane(*screen, framebuffer, damage); Ok(buf.len()) } @@ -255,8 +260,8 @@ impl Scheme for GraphicsScheme { log::info!("KSMSG MMAP {} {:?} {} {}", id, flags, offset, size); let handle = self.handles.get(&id).ok_or(Error::new(EINVAL))?; let Handle::Screen { vt, screen } = handle; - let resource = &self.vts_res[vt][screen]; - let ptr = T::map_resource(&mut self.adapter, resource); + let framebuffer = &self.vts_fb[vt][screen]; + let ptr = T::map_dumb_framebuffer(&mut self.adapter, framebuffer); Ok(ptr as usize) } } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 938b8a6678..32e3a22436 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -2,7 +2,7 @@ use std::alloc::{self, Layout}; use std::convert::TryInto; use std::ptr::{self, NonNull}; -use driver_graphics::{GraphicsAdapter, Resource}; +use driver_graphics::{Framebuffer, GraphicsAdapter}; use graphics_ipc::v1::Damage; use syscall::PAGE_SIZE; @@ -13,7 +13,7 @@ pub struct FbAdapter { } impl GraphicsAdapter for FbAdapter { - type Resource = GraphicScreen; + type Framebuffer = GraphicScreen; fn displays(&self) -> Vec { (0..self.framebuffers.len()).collect() @@ -26,16 +26,21 @@ impl GraphicsAdapter for FbAdapter { ) } - fn create_resource(&mut self, width: u32, height: u32) -> Self::Resource { + fn create_dumb_framebuffer(&mut self, width: u32, height: u32) -> Self::Framebuffer { GraphicScreen::new(width as usize, height as usize) } - fn map_resource(&mut self, resource: &Self::Resource) -> *mut u8 { - resource.ptr.as_ptr().cast::() + fn map_dumb_framebuffer(&mut self, framebuffer: &Self::Framebuffer) -> *mut u8 { + framebuffer.ptr.as_ptr().cast::() } - fn update_plane(&mut self, display_id: usize, resource: &Self::Resource, damage: &[Damage]) { - resource.sync(&mut self.framebuffers[display_id], damage) + fn update_plane( + &mut self, + display_id: usize, + framebuffer: &Self::Framebuffer, + damage: &[Damage], + ) { + framebuffer.sync(&mut self.framebuffers[display_id], damage) } } @@ -73,7 +78,7 @@ impl Drop for GraphicScreen { } } -impl Resource for GraphicScreen { +impl Framebuffer for GraphicScreen { fn width(&self) -> u32 { self.width as u32 } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 6bd41c91a7..b6eda2e2b0 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use common::{dma::Dma, sgl}; -use driver_graphics::{GraphicsAdapter, GraphicsScheme, Resource}; +use driver_graphics::{Framebuffer, GraphicsAdapter, GraphicsScheme}; use graphics_ipc::v1::Damage; use inputd::DisplayHandle; @@ -23,14 +23,14 @@ impl Into for Damage { } } -pub struct VirtGpuResource { +pub struct VirtGpuFramebuffer { id: ResourceId, sgl: sgl::Sgl, width: u32, height: u32, } -impl Resource for VirtGpuResource { +impl Framebuffer for VirtGpuFramebuffer { fn width(&self) -> u32 { self.width } @@ -95,7 +95,7 @@ impl VirtGpuAdapter<'_> { } impl GraphicsAdapter for VirtGpuAdapter<'_> { - type Resource = VirtGpuResource; + type Framebuffer = VirtGpuFramebuffer; fn displays(&self) -> Vec { self.displays.iter().enumerate().map(|(i, _)| i).collect() @@ -108,7 +108,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { ) } - fn create_resource(&mut self, width: u32, height: u32) -> Self::Resource { + fn create_dumb_framebuffer(&mut self, width: u32, height: u32) -> Self::Framebuffer { futures::executor::block_on(async { let bpp = 32; let fb_size = width as usize * height as usize * bpp / 8; @@ -157,7 +157,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { self.control_queue.send(command).await; assert_eq!(header.ty, CommandTy::RespOkNodata); - VirtGpuResource { + VirtGpuFramebuffer { id: res_id, sgl, width, @@ -166,19 +166,24 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { }) } - fn map_resource(&mut self, resource: &Self::Resource) -> *mut u8 { - resource.sgl.as_ptr() + fn map_dumb_framebuffer(&mut self, framebuffer: &Self::Framebuffer) -> *mut u8 { + framebuffer.sgl.as_ptr() } - fn update_plane(&mut self, display_id: usize, resource: &Self::Resource, damage: &[Damage]) { + fn update_plane( + &mut self, + display_id: usize, + framebuffer: &Self::Framebuffer, + damage: &[Damage], + ) { futures::executor::block_on(async { let req = Dma::new(XferToHost2d::new( - resource.id, + framebuffer.id, GpuRect { x: 0, y: 0, - width: resource.width, - height: resource.height, + width: framebuffer.width, + height: framebuffer.height, }, 0, )) @@ -187,22 +192,22 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { assert_eq!(header.ty, CommandTy::RespOkNodata); // FIXME once we support resizing we also need to check that the current and target size match - if self.displays[display_id].active_resource != Some(resource.id) { + if self.displays[display_id].active_resource != Some(framebuffer.id) { let scanout_request = Dma::new(SetScanout::new( display_id as u32, - resource.id, - GpuRect::new(0, 0, resource.width, resource.height), + framebuffer.id, + GpuRect::new(0, 0, framebuffer.width, framebuffer.height), )) .unwrap(); let header = self.send_request(scanout_request).await.unwrap(); assert_eq!(header.ty, CommandTy::RespOkNodata); - self.displays[display_id].active_resource = Some(resource.id); + self.displays[display_id].active_resource = Some(framebuffer.id); } for damage in damage { let flush = ResourceFlush::new( - resource.id, - damage.clip(resource.width, resource.height).into(), + framebuffer.id, + damage.clip(framebuffer.width, framebuffer.height).into(), ); let header = self.send_request(Dma::new(flush).unwrap()).await.unwrap(); assert_eq!(header.ty, CommandTy::RespOkNodata);