GPU: How can the framebuffer be reused?

[tpdenk]

virtio-drivers/src/device/gpu.rs

Lines 118 to 124 in 0d59b01

	// SAFETY: `Dma::new` guarantees that the pointer returned from
	// `raw_slice` is non-null, aligned, and the allocation is zeroed. We
	// store the `Dma` object in `self.frame_buffer_dma`, which prevents the
	// allocation from being freed while `self` exists. The returned ptr
	// borrows `self` mutably, which prevents other code from getting
	// another reference to `frame_buffer_dma` while the returned slice is
	// still in use.

This is accurate, but this also prevents me from reusing the framebuffer that was set up. From the example:

    let fb = gpu.setup_framebuffer().expect("failed to get fb");
    for y in 0..height {
        for x in 0..width {
            let idx = (y * width + x) * 4;
            fb[idx] = x as u8;
            fb[idx + 1] = y as u8;
            fb[idx + 2] = (x + y) as u8;
        }
    }
    gpu.flush().expect("failed to flush");

I can't update the framebuffer once I've flushed, but the only way to get another framebuffer is to call setup_framebuffer again, which does a full framebuffer allocation again.

(so the following wouldn't compile)

let fb = gpu.setup_framebuffer()?;
fb.fill(0xaa);
gpu.flush()?;
fb.fill(0xbb);
gpu.flush()?;

How was the framebuffer intended to be used? Am I missing something? Do I need to manually track the allocations from the Hal?

[samuelh2005]

What you should do is create a wrapper struct that holds the framebuffer / GPU then you can use a getter function to retrive the framebuffer as many times as you want.

[tpdenk]

I don't see how that would work. setup_framebuffer derives a mutable reference from the gpu. As long as that reference is alive, I can't call flush.

Do you have a working example of what you're suggesting?

[samuelh2005]

I don't see how that would work. setup_framebuffer derives a mutable reference from the gpu. As long as that reference is alive, I can't call flush.

Do you have a working example of what you're suggesting?

In my OS I use this:

use virtio_drivers::device::gpu::VirtIOGpu;
use virtio_drivers::transport::Transport;

use crate::device::DeviceInfo;
use crate::platform::hal::HalImpl;

/// VirtIO GPU device with a generic transport  
pub struct VirtioGpuDevice<T: Transport, I: DeviceInfo> {
    gpu: VirtIOGpu<HalImpl, T>,
    info: I,
    framebuffer: Option<&'static mut [u8]>,
}

impl<T: Transport, I: DeviceInfo> VirtioGpuDevice<T, I> {
    /// Create a new VirtIO GPU device with a generic transport
    pub fn new(transport: T, device_info: I) -> Result<Self, virtio_drivers::Error> {
        let gpu = VirtIOGpu::new(transport)?;
        Ok(VirtioGpuDevice {
            gpu,
            info: device_info,
            framebuffer: None,
        })
    }

    /// Get a reference to the underlying VirtIOGpu driver
    pub fn driver(&mut self) -> &mut VirtIOGpu<HalImpl, T> {
        &mut self.gpu
    }

    /// Setup the framebuffer (call this once)
    pub fn setup_framebuffer(&mut self) -> Result<(), virtio_drivers::Error> {
        if self.framebuffer.is_none() {
            let fb = self.gpu.setup_framebuffer()?;
            self.framebuffer = Some(unsafe { core::mem::transmute(fb) });
        }
        Ok(())
    }

    /// Get a mutable reference to the framebuffer
    pub fn framebuffer(&mut self) -> Result<&mut [u8], virtio_drivers::Error> {
        match &mut self.framebuffer {
            Some(fb) => Ok(fb),
            None => Err(virtio_drivers::Error::InvalidParam),
        }
    }

    /// Flush the framebuffer to the screen
    pub fn flush(&mut self) -> Result<(), virtio_drivers::Error> {
        self.gpu.flush()
    }

    /// Get the screen resolution
    pub fn resolution(&mut self) -> Result<(u32, u32), virtio_drivers::Error> {
        self.gpu.resolution()
    }

    /// Get the drawable resolution
    pub fn drawable_resolution(&mut self) -> Result<(u32, u32), virtio_drivers::Error> {
        self.gpu.drawable_resolution()
    }

    /// Get device info
    pub fn device_info(&self) -> &I {
        &self.info
    }
}

use crate::device::DeviceImpl;

impl<T: Transport, I:DeviceInfo> DeviceImpl for VirtioGpuDevice<T, I> {
    type Info = I;
    
    fn info(&self) -> &Self::Info {
        &self.info
    }
}

You don't need the device or device info traits but if you want a generic device manager they are a good option.

[tpdenk]

That's UB, but maybe a good starting point. It's just a really big footgun.

Thank you anyways.