From ed9ad1aaf167d192d1e5d9f8409a202a18014a73 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:57:18 +0100 Subject: [PATCH 1/7] graphics/vesad: Remove old resizing code Graphics clients will need to handle resizing themself anyway for handoff between graphics drivers and as such don't need the graphics to modify the framebuffer during resize. --- graphics/vesad/src/framebuffer.rs | 34 ---------------------- graphics/vesad/src/screen.rs | 47 +------------------------------ 2 files changed, 1 insertion(+), 80 deletions(-) diff --git a/graphics/vesad/src/framebuffer.rs b/graphics/vesad/src/framebuffer.rs index 74d5b1be89..7eeab2fe4d 100644 --- a/graphics/vesad/src/framebuffer.rs +++ b/graphics/vesad/src/framebuffer.rs @@ -1,7 +1,5 @@ use std::ptr; -use syscall::PAGE_SIZE; - pub struct FrameBuffer { pub onscreen: *mut [u32], pub phys: usize, @@ -58,36 +56,4 @@ impl FrameBuffer { let stride = parse_number(parts.next()?)?; Some(Self::new(phys, width, height, stride)) } - - pub unsafe fn resize(&mut self, width: usize, height: usize, stride: usize) { - // Unmap old onscreen - unsafe { - let slice = self.onscreen; - libredox::call::munmap(slice.cast(), (slice.len() * 4).next_multiple_of(PAGE_SIZE)) - .expect("vesad: failed to unmap framebuffer"); - } - - // Map new onscreen - self.onscreen = unsafe { - let size = stride * height; - let onscreen_ptr = common::physmap( - self.phys, - size * 4, - common::Prot { - read: true, - write: true, - }, - common::MemoryType::WriteCombining, - ) - .expect("vesad: failed to map framebuffer") as *mut u32; - ptr::write_bytes(onscreen_ptr, 0, size); - - ptr::slice_from_raw_parts_mut(onscreen_ptr, size) - }; - - // Update size - self.width = width; - self.height = height; - self.stride = stride; - } } diff --git a/graphics/vesad/src/screen.rs b/graphics/vesad/src/screen.rs index 71b99f7b51..3523f24a83 100644 --- a/graphics/vesad/src/screen.rs +++ b/graphics/vesad/src/screen.rs @@ -1,5 +1,5 @@ use std::convert::TryInto; -use std::{cmp, ptr}; +use std::ptr; use driver_graphics::Resource; use inputd::Damage; @@ -34,51 +34,6 @@ impl Resource for GraphicScreen { } impl GraphicScreen { - pub fn resize(&mut self, width: usize, height: usize) { - //TODO: Fix issue with mapped screens - - if width != self.width || height != self.height { - println!("Resize display to {}, {}", width, height); - - let size = width * height; - let mut offscreen = OffscreenBuffer::new(size); - - let mut old_ptr = self.offscreen.as_ptr(); - let mut new_ptr = offscreen.as_mut_ptr(); - - for _y in 0..cmp::min(height, self.height) { - unsafe { - ptr::copy(old_ptr, new_ptr, cmp::min(width, self.width)); - if width > self.width { - ptr::write_bytes( - new_ptr.offset(self.width as isize), - 0, - width - self.width, - ); - } - old_ptr = old_ptr.offset(self.width as isize); - new_ptr = new_ptr.offset(width as isize); - } - } - - if height > self.height { - for _y in self.height..height { - unsafe { - ptr::write_bytes(new_ptr, 0, width); - new_ptr = new_ptr.offset(width as isize); - } - } - } - - self.width = width; - self.height = height; - - self.offscreen = offscreen; - } else { - println!("Display is already {}, {}", width, height); - }; - } - pub fn sync(&self, framebuffer: &mut FrameBuffer, sync_rects: &[Damage]) { for sync_rect in sync_rects { let sync_rect = sync_rect.clip( From 9afe3b390baf25d88d5adbebfe7552457e717f67 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 27 Dec 2024 15:57:43 +0100 Subject: [PATCH 2/7] graphics/vesad: Remove all usage of unstable features --- graphics/vesad/src/display.rs | 2 +- graphics/vesad/src/main.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/graphics/vesad/src/display.rs b/graphics/vesad/src/display.rs index 8aad04d62c..f1276bcdd4 100644 --- a/graphics/vesad/src/display.rs +++ b/graphics/vesad/src/display.rs @@ -23,7 +23,7 @@ impl OffscreenBuffer { } pub fn ptr(&self) -> *mut u8 { - self.ptr.as_mut_ptr().cast::() + self.ptr.as_ptr().cast::() } } impl Drop for OffscreenBuffer { diff --git a/graphics/vesad/src/main.rs b/graphics/vesad/src/main.rs index 457870ba64..e92f92b642 100644 --- a/graphics/vesad/src/main.rs +++ b/graphics/vesad/src/main.rs @@ -1,4 +1,3 @@ -#![feature(int_roundings, slice_ptr_get)] extern crate orbclient; extern crate syscall; From 1d89ca7bd2ed3287bc1ea935cde908ee98a8a105 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 28 Dec 2024 14:01:09 +0100 Subject: [PATCH 3/7] graphics/virtio-gpud: Remove unnecessary VolatileCell The virtio queue abstraction already provides the necessary synchronization to make everything data-race free without volatile accesses. --- graphics/virtio-gpud/src/main.rs | 79 ++++++++++-------------------- graphics/virtio-gpud/src/scheme.rs | 28 +++++------ 2 files changed, 39 insertions(+), 68 deletions(-) diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index 985d03b7c1..f6084b876c 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -22,7 +22,6 @@ #![feature(int_roundings)] -use std::cell::UnsafeCell; use std::os::fd::AsRawFd; use std::sync::atomic::{AtomicU32, Ordering}; @@ -41,22 +40,16 @@ macro_rules! make_getter_setter { ($($field:ident: $return_ty:ty),*) => { $( pub fn $field(&self) -> $return_ty { - self.$field.get() + self.$field } paste::item! { pub fn [](&mut self, value: $return_ty) { - self.$field.set(value) + self.$field = value; } } )* }; - - (@$field:ident: $return_ty:ty) => { - pub fn $field(&mut self, value: $return_ty) { - self.$field.set(value) - } - }; } #[repr(C)] @@ -137,18 +130,18 @@ static_assertions::const_assert_eq!(core::mem::size_of::(), 4); #[derive(Debug)] #[repr(C)] pub struct ControlHeader { - pub ty: VolatileCell, - pub flags: VolatileCell, - pub fence_id: VolatileCell, - pub ctx_id: VolatileCell, - pub ring_index: VolatileCell, + pub ty: CommandTy, + pub flags: u32, + pub fence_id: u64, + pub ctx_id: u32, + pub ring_index: u8, padding: [u8; 3], } impl ControlHeader { pub fn with_ty(ty: CommandTy) -> Self { Self { - ty: VolatileCell::new(ty), + ty, ..Default::default() } } @@ -157,11 +150,11 @@ impl ControlHeader { impl Default for ControlHeader { fn default() -> Self { Self { - ty: VolatileCell::new(CommandTy::Undefined), - flags: VolatileCell::new(0), - fence_id: VolatileCell::new(0), - ctx_id: VolatileCell::new(0), - ring_index: VolatileCell::new(0), + ty: CommandTy::Undefined, + flags: 0, + fence_id: 0, + ctx_id: 0, + ring_index: 0, padding: [0; 3], } } @@ -190,16 +183,9 @@ impl GpuRect { #[derive(Debug)] #[repr(C)] pub struct DisplayInfo { - rect: UnsafeCell, - pub enabled: VolatileCell, - pub flags: VolatileCell, -} - -impl DisplayInfo { - pub fn rect(&self) -> &GpuRect { - // SAFETY: We never give out mutable references to `self.rect`. - unsafe { &*self.rect.get() } - } + rect: GpuRect, + pub enabled: u32, + pub flags: u32, } #[derive(Debug)] @@ -213,7 +199,7 @@ impl Default for GetDisplayInfo { fn default() -> Self { Self { header: ControlHeader { - ty: VolatileCell::new(CommandTy::GetDisplayInfo), + ty: CommandTy::GetDisplayInfo, ..Default::default() }, @@ -247,13 +233,10 @@ pub enum ResourceFormat { #[repr(C)] pub struct ResourceCreate2d { pub header: ControlHeader, - - // FIXME we can likely use regular loads and stores as the ring buffer should provide the - // necessary synchronization. - resource_id: VolatileCell, - format: VolatileCell, - width: VolatileCell, - height: VolatileCell, + resource_id: ResourceId, + format: ResourceFormat, + width: u32, + height: u32, } impl ResourceCreate2d { @@ -263,15 +246,11 @@ impl ResourceCreate2d { impl Default for ResourceCreate2d { fn default() -> Self { Self { - header: ControlHeader { - ty: VolatileCell::new(CommandTy::ResourceCreate2d), - ..Default::default() - }, - - resource_id: VolatileCell::new(ResourceId(0)), - format: VolatileCell::new(ResourceFormat::Unknown), - width: VolatileCell::new(0), - height: VolatileCell::new(0), + header: ControlHeader::with_ty(CommandTy::ResourceCreate2d), + resource_id: ResourceId(0), + format: ResourceFormat::Unknown, + width: 0, + height: 0, } } } @@ -392,11 +371,7 @@ pub struct XferToHost2d { impl XferToHost2d { pub fn new(resource_id: ResourceId, rect: GpuRect, offset: u64) -> Self { Self { - header: ControlHeader { - ty: VolatileCell::new(CommandTy::TransferToHost2d), - ..Default::default() - }, - + header: ControlHeader::with_ty(CommandTy::TransferToHost2d), rect, offset, resource_id, diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index db89ee0f02..cc9c9a86bd 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -8,7 +8,6 @@ use syscall::PAGE_SIZE; use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags}; use virtio_core::transport::{Error, Queue, Transport}; -use virtio_core::utils::VolatileCell; use crate::*; @@ -67,7 +66,7 @@ impl VirtGpuAdapter<'_> { async fn flush_resource_inner(&self, flush: ResourceFlush) -> Result<(), Error> { let header = self.send_request(Dma::new(flush)?).await?; - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); + assert_eq!(header.ty, CommandTy::RespOkNodata); Ok(()) } @@ -108,7 +107,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { request.set_resource_id(res_id); let header = self.send_request(request).await.unwrap(); - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); + assert_eq!(header.ty, CommandTy::RespOkNodata); // Use the allocated framebuffer from tthe guest ram, and attach it as backing // storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. @@ -133,7 +132,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { .build(); self.control_queue.send(command).await; - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); + assert_eq!(header.ty, CommandTy::RespOkNodata); VirtGpuResource { id: res_id, @@ -162,7 +161,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { )) .unwrap(); let header = self.send_request(scanout_request).await.unwrap(); - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); + assert_eq!(header.ty, CommandTy::RespOkNodata); }); self.flush_resource(display_id, resource, None); @@ -187,7 +186,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { )) .unwrap(); let header = self.send_request(req).await.unwrap(); - assert_eq!(header.ty.get(), CommandTy::RespOkNodata); + assert_eq!(header.ty, CommandTy::RespOkNodata); if let Some(damage) = damage { for damage in damage { @@ -259,11 +258,11 @@ impl<'a> GpuScheme<'a> { for info in displays.iter() { log::info!( "virtio-gpu: opening display ({}x{}px)", - info.rect().width, - info.rect().height + info.rect.width, + info.rect.height ); - if info.rect().width == 0 || info.rect().height == 0 { + if info.rect.width == 0 || info.rect.height == 0 { // QEMU gives all displays other than the first a zero width and height, but trying // to attach a zero sized framebuffer to the display will result an error, so // default to 640x480px. @@ -273,8 +272,8 @@ impl<'a> GpuScheme<'a> { }); } else { result.push(Display { - width: info.rect().width, - height: info.rect().height, + width: info.rect.width, + height: info.rect.height, }); } } @@ -283,10 +282,7 @@ impl<'a> GpuScheme<'a> { } async fn get_display_info(control_queue: Arc>) -> Result, Error> { - let header = Dma::new(ControlHeader { - ty: VolatileCell::new(CommandTy::GetDisplayInfo), - ..Default::default() - })?; + let header = Dma::new(ControlHeader::with_ty(CommandTy::GetDisplayInfo))?; let response = Dma::new(GetDisplayInfo::default())?; let command = ChainBuilder::new() @@ -295,7 +291,7 @@ impl<'a> GpuScheme<'a> { .build(); control_queue.send(command).await; - assert!(response.header.ty.get() == CommandTy::RespOkDisplayInfo); + assert!(response.header.ty == CommandTy::RespOkDisplayInfo); Ok(response) } From 1ef298e5767efbe5045b37820099a92e07e54b51 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 28 Dec 2024 14:04:10 +0100 Subject: [PATCH 4/7] graphics/virtio-gpud: Remove getters and setters for ResourceCreate2d --- Cargo.lock | 7 ------- graphics/virtio-gpud/Cargo.toml | 1 - graphics/virtio-gpud/src/main.rs | 30 +++++------------------------- graphics/virtio-gpud/src/scheme.rs | 13 +++++++------ 4 files changed, 12 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28de7e560d..374df5ce0a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -941,12 +941,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "paste" -version = "1.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" - [[package]] name = "paw" version = "1.0.0" @@ -1778,7 +1772,6 @@ dependencies = [ "libredox", "log", "orbclient", - "paste", "pcid", "redox-daemon", "redox_event", diff --git a/graphics/virtio-gpud/Cargo.toml b/graphics/virtio-gpud/Cargo.toml index 42da6e798f..a8cf34ed40 100644 --- a/graphics/virtio-gpud/Cargo.toml +++ b/graphics/virtio-gpud/Cargo.toml @@ -9,7 +9,6 @@ log = "0.4" static_assertions = "1.1.0" futures = { version = "0.3.28", features = ["executor"] } anyhow = "1.0.71" -paste = "1.0.13" common = { path = "../../common" } driver-graphics = { path = "../driver-graphics" } diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index f6084b876c..9ce2447ba0 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -36,22 +36,6 @@ mod scheme; // const VIRTIO_GPU_EVENT_DISPLAY: u32 = 1 << 0; const VIRTIO_GPU_MAX_SCANOUTS: usize = 16; -macro_rules! make_getter_setter { - ($($field:ident: $return_ty:ty),*) => { - $( - pub fn $field(&self) -> $return_ty { - self.$field - } - - paste::item! { - pub fn [](&mut self, value: $return_ty) { - self.$field = value; - } - } - )* - }; -} - #[repr(C)] pub struct GpuConfig { /// Signals pending events to the driver. @@ -240,17 +224,13 @@ pub struct ResourceCreate2d { } impl ResourceCreate2d { - make_getter_setter!(resource_id: ResourceId, format: ResourceFormat, width: u32, height: u32); -} - -impl Default for ResourceCreate2d { - fn default() -> Self { + fn new(resource_id: ResourceId, format: ResourceFormat, width: u32, height: u32) -> Self { Self { header: ControlHeader::with_ty(CommandTy::ResourceCreate2d), - resource_id: ResourceId(0), - format: ResourceFormat::Unknown, - width: 0, - height: 0, + resource_id, + format, + width, + height, } } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index cc9c9a86bd..7a548407d1 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -99,12 +99,13 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { let res_id = ResourceId::alloc(); // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. - let mut request = Dma::new(ResourceCreate2d::default()).unwrap(); - - request.set_width(width); - request.set_height(height); - request.set_format(ResourceFormat::Bgrx); - request.set_resource_id(res_id); + let request = Dma::new(ResourceCreate2d::new( + res_id, + ResourceFormat::Bgrx, + width, + height, + )) + .unwrap(); let header = self.send_request(request).await.unwrap(); assert_eq!(header.ty, CommandTy::RespOkNodata); From 0b8648ff5ba3eb90da1b1e4b606b2daf16c04733 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 28 Dec 2024 14:16:56 +0100 Subject: [PATCH 5/7] graphics/virtio-gpud: Move some initialization code around --- graphics/virtio-gpud/src/main.rs | 12 ++--- graphics/virtio-gpud/src/scheme.rs | 85 ++++++++++++------------------ 2 files changed, 40 insertions(+), 57 deletions(-) diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index 9ce2447ba0..880e80b9ab 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -392,7 +392,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { device.transport.run_device(); deamon.ready().unwrap(); - let mut scheme = futures::executor::block_on(scheme::GpuScheme::new( + let (mut scheme, mut inputd_handle) = futures::executor::block_on(scheme::GpuScheme::new( config, control_queue.clone(), cursor_queue.clone(), @@ -410,14 +410,14 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { EventQueue::new().expect("virtio-gpud: failed to create event queue"); event_queue .subscribe( - scheme.inputd_handle.inner().as_raw_fd() as usize, + inputd_handle.inner().as_raw_fd() as usize, Source::Input, event::EventFlags::READ, ) .unwrap(); event_queue .subscribe( - scheme.inner.event_handle().raw(), + scheme.event_handle().raw(), Source::Scheme, event::EventFlags::READ, ) @@ -430,17 +430,15 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { { match event { Source::Input => { - while let Some(vt_event) = scheme - .inputd_handle + while let Some(vt_event) = inputd_handle .read_vt_event() .expect("virtio-gpud: failed to read display handle") { - scheme.inner.handle_vt_event(vt_event); + scheme.handle_vt_event(vt_event); } } Source::Scheme => { scheme - .inner .tick() .expect("virtio-gpud: failed to process scheme events"); } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 7a548407d1..82809cad41 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 inputd::Damage; +use inputd::{Damage, DisplayHandle}; use syscall::PAGE_SIZE; @@ -70,6 +70,21 @@ impl VirtGpuAdapter<'_> { Ok(()) } + + async fn get_display_info(&self) -> Result, Error> { + let header = Dma::new(ControlHeader::with_ty(CommandTy::GetDisplayInfo))?; + + let response = Dma::new(GetDisplayInfo::default())?; + let command = ChainBuilder::new() + .chain(Buffer::new(&header)) + .chain(Buffer::new(&response).flags(DescriptorFlags::WRITE_ONLY)) + .build(); + + self.control_queue.send(command).await; + assert!(response.header.ty == CommandTy::RespOkDisplayInfo); + + Ok(response) + } } impl GraphicsAdapter for VirtGpuAdapter<'_> { @@ -217,46 +232,26 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { } } -pub struct GpuScheme<'a> { - pub inner: GraphicsScheme>, - pub inputd_handle: inputd::DisplayHandle, -} +pub struct GpuScheme {} -impl<'a> GpuScheme<'a> { +impl<'a> GpuScheme { pub async fn new( config: &'a mut GpuConfig, control_queue: Arc>, cursor_queue: Arc>, transport: Arc, - ) -> Result, Error> { - let displays = Self::probe(control_queue.clone(), config).await?; + ) -> Result<(GraphicsScheme>, DisplayHandle), Error> { + let mut adapter = VirtGpuAdapter { + control_queue, + cursor_queue, + transport, + displays: vec![], + }; - let inputd_handle = inputd::DisplayHandle::new("virtio-gpu").unwrap(); + let mut display_info = adapter.get_display_info().await?; + let raw_displays = &mut display_info.display_info[..config.num_scanouts() as usize]; - Ok(Self { - inner: GraphicsScheme::new( - VirtGpuAdapter { - control_queue, - cursor_queue, - transport, - displays, - }, - "display.virtio-gpu".to_owned(), - ), - inputd_handle, - }) - } - - async fn probe( - control_queue: Arc>, - config: &GpuConfig, - ) -> Result, Error> { - let mut display_info = Self::get_display_info(control_queue.clone()).await?; - let displays = &mut display_info.display_info[..config.num_scanouts() as usize]; - - let mut result = vec![]; - - for info in displays.iter() { + for info in raw_displays.iter() { log::info!( "virtio-gpu: opening display ({}x{}px)", info.rect.width, @@ -267,33 +262,23 @@ impl<'a> GpuScheme<'a> { // QEMU gives all displays other than the first a zero width and height, but trying // to attach a zero sized framebuffer to the display will result an error, so // default to 640x480px. - result.push(Display { + adapter.displays.push(Display { width: 640, height: 480, }); } else { - result.push(Display { + adapter.displays.push(Display { width: info.rect.width, height: info.rect.height, }); } } - Ok(result) - } + let inputd_handle = DisplayHandle::new("virtio-gpu").unwrap(); - async fn get_display_info(control_queue: Arc>) -> Result, Error> { - let header = Dma::new(ControlHeader::with_ty(CommandTy::GetDisplayInfo))?; - - let response = Dma::new(GetDisplayInfo::default())?; - let command = ChainBuilder::new() - .chain(Buffer::new(&header)) - .chain(Buffer::new(&response).flags(DescriptorFlags::WRITE_ONLY)) - .build(); - - control_queue.send(command).await; - assert!(response.header.ty == CommandTy::RespOkDisplayInfo); - - Ok(response) + Ok(( + GraphicsScheme::new(adapter, "display.virtio-gpu".to_owned()), + inputd_handle, + )) } } From b5eeaed9e17ce2acaf4a50f01c4e90718d62516f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 28 Dec 2024 14:31:03 +0100 Subject: [PATCH 6/7] graphics/virtio-gpud: Use resource size rather than display size in set_scanout This ensures we use the right size if the display got resized but the resource hasn't been resized yet. --- graphics/virtio-gpud/src/scheme.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 82809cad41..3f140fb8f3 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -125,7 +125,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { let header = self.send_request(request).await.unwrap(); assert_eq!(header.ty, CommandTy::RespOkNodata); - // Use the allocated framebuffer from tthe guest ram, and attach it as backing + // Use the allocated framebuffer from the guest ram, and attach it as backing // storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. let mut mem_entries = @@ -168,12 +168,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { let scanout_request = Dma::new(SetScanout::new( display_id as u32, resource.id, - GpuRect::new( - 0, - 0, - self.displays[display_id].width, - self.displays[display_id].height, - ), + GpuRect::new(0, 0, resource.width, resource.height), )) .unwrap(); let header = self.send_request(scanout_request).await.unwrap(); From 5af43fcaa2c2ecaece4d029f42bc4c12bc0a95aa Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 28 Dec 2024 14:51:02 +0100 Subject: [PATCH 7/7] graphics/vesad: Merge OffscreenBuffer into GraphicScreen --- graphics/vesad/src/display.rs | 45 ----------------------------------- graphics/vesad/src/main.rs | 1 - graphics/vesad/src/scheme.rs | 2 +- graphics/vesad/src/screen.rs | 41 ++++++++++++++++++++++++------- 4 files changed, 33 insertions(+), 56 deletions(-) delete mode 100644 graphics/vesad/src/display.rs diff --git a/graphics/vesad/src/display.rs b/graphics/vesad/src/display.rs deleted file mode 100644 index f1276bcdd4..0000000000 --- a/graphics/vesad/src/display.rs +++ /dev/null @@ -1,45 +0,0 @@ -use std::alloc::{self, Layout}; -use std::ptr; -use std::ptr::NonNull; - -pub struct OffscreenBuffer { - ptr: NonNull<[u32]>, -} - -impl OffscreenBuffer { - #[inline] - fn layout(len: usize) -> Layout { - // optimizes to an integer mul - Layout::array::(len).unwrap().align_to(4096).unwrap() - } - - #[inline] - pub fn new(len: usize) -> Self { - let layout = Self::layout(len); - let ptr = unsafe { alloc::alloc_zeroed(layout) }; - let ptr = ptr::slice_from_raw_parts_mut(ptr.cast(), len); - let ptr = NonNull::new(ptr).unwrap_or_else(|| alloc::handle_alloc_error(layout)); - OffscreenBuffer { ptr } - } - - pub fn ptr(&self) -> *mut u8 { - self.ptr.as_ptr().cast::() - } -} -impl Drop for OffscreenBuffer { - fn drop(&mut self) { - let layout = Self::layout(self.ptr.len()); - unsafe { alloc::dealloc(self.ptr.as_ptr().cast(), layout) }; - } -} -impl std::ops::Deref for OffscreenBuffer { - type Target = [u32]; - fn deref(&self) -> &[u32] { - unsafe { self.ptr.as_ref() } - } -} -impl std::ops::DerefMut for OffscreenBuffer { - fn deref_mut(&mut self) -> &mut [u32] { - unsafe { self.ptr.as_mut() } - } -} diff --git a/graphics/vesad/src/main.rs b/graphics/vesad/src/main.rs index e92f92b642..8cafa6b2ee 100644 --- a/graphics/vesad/src/main.rs +++ b/graphics/vesad/src/main.rs @@ -11,7 +11,6 @@ use std::os::fd::AsRawFd; use crate::framebuffer::FrameBuffer; use crate::scheme::FbAdapter; -mod display; mod framebuffer; mod scheme; mod screen; diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 495a5a48cf..15d7f2867a 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -25,7 +25,7 @@ impl GraphicsAdapter for FbAdapter { } fn map_resource(&mut self, resource: &Self::Resource) -> *mut u8 { - resource.offscreen.ptr() + resource.ptr() } fn set_scanout(&mut self, display_id: usize, resource: &Self::Resource) { diff --git a/graphics/vesad/src/screen.rs b/graphics/vesad/src/screen.rs index 3523f24a83..e90a4b39c2 100644 --- a/graphics/vesad/src/screen.rs +++ b/graphics/vesad/src/screen.rs @@ -1,25 +1,44 @@ +use std::alloc::{self, Layout}; use std::convert::TryInto; -use std::ptr; +use std::ptr::{self, NonNull}; use driver_graphics::Resource; use inputd::Damage; +use syscall::PAGE_SIZE; -use crate::display::OffscreenBuffer; use crate::framebuffer::FrameBuffer; pub struct GraphicScreen { pub width: usize, pub height: usize, - pub offscreen: OffscreenBuffer, + ptr: NonNull<[u32]>, } impl GraphicScreen { pub fn new(width: usize, height: usize) -> GraphicScreen { - GraphicScreen { - width, - height, - offscreen: OffscreenBuffer::new(width * height), - } + let len = width * height; + let layout = Self::layout(len); + let ptr = unsafe { alloc::alloc_zeroed(layout) }; + let ptr = ptr::slice_from_raw_parts_mut(ptr.cast(), len); + let ptr = NonNull::new(ptr).unwrap_or_else(|| alloc::handle_alloc_error(layout)); + + GraphicScreen { width, height, ptr } + } + + #[inline] + fn layout(len: usize) -> Layout { + // optimizes to an integer mul + Layout::array::(len) + .unwrap() + .align_to(PAGE_SIZE) + .unwrap() + } +} + +impl Drop for GraphicScreen { + fn drop(&mut self) { + let layout = Self::layout(self.ptr.len()); + unsafe { alloc::dealloc(self.ptr.as_ptr().cast(), layout) }; } } @@ -34,6 +53,10 @@ impl Resource for GraphicScreen { } impl GraphicScreen { + pub fn ptr(&self) -> *mut u8 { + self.ptr.as_ptr().cast::() + } + pub fn sync(&self, framebuffer: &mut FrameBuffer, sync_rects: &[Damage]) { for sync_rect in sync_rects { let sync_rect = sync_rect.clip( @@ -50,7 +73,7 @@ impl GraphicScreen { let row_pixel_count = w; - let mut offscreen_ptr = self.offscreen.as_ptr(); + let mut offscreen_ptr = self.ptr.as_ptr() as *mut u32; let mut onscreen_ptr = framebuffer.onscreen as *mut u32; // FIXME use as_mut_ptr once stable unsafe {