Re: [PATCH 2/4] ramfb: Add driver for ramfb display

Simon Glass
2022-03-12T02:31:56Z

Hi Alex,

On Sun, 27 Feb 2022 at 07:40, Alexander Graf <agraf@csgraf.de> wrote:
>
> QEMU implements multiple ways to expose graphics output to the virt
> machine, but most of them are incompatible with hardware virtualization.
>
> The one that does work reliably is ramfb. It's a very simple mechanism
> in which the guest reserves a memory region for the frame buffer and then
> notifies the host about its location and properties. The host then just
> displays the contents of the frame buffer on screen.
>
> This patch implements a trivial version of a ramfb driver - hard coded
> to a single resolution.
>
> Signed-off-by: Alexander Graf <agraf@csgraf.de>
> ---
> drivers/video/Kconfig | 8 +++
> drivers/video/MAINTAINERS | 4 ++
> drivers/video/Makefile | 1 +
> drivers/video/ramfb.c | 104 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 117 insertions(+)
> create mode 100644 drivers/video/MAINTAINERS
> create mode 100644 drivers/video/ramfb.c
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index ff8e11f648..73a9e20534 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -871,6 +871,14 @@ config VIDEO_MCDE_SIMPLE
> before u-boot starts, and u-boot will simply render to the pre-
> allocated frame buffer surface.
>
> +config VIDEO_RAMFB
> + bool "QEMU ramfb display driver for in-RAM display"
> + depends on EFI_LOADER && DM_VIDEO && QFW
> + help
> + Enables a RAM based simple frame buffer driver which uses qfw to
> + notify the hypervisor about the location of a Frame Buffer allocated
> + in guest RAM as well as its properties.
> +
> config OSD
> bool "Enable OSD support"
> depends on DM
> diff --git a/drivers/video/MAINTAINERS b/drivers/video/MAINTAINERS
> new file mode 100644
> index 0000000000..74c258a314
> --- /dev/null
> +++ b/drivers/video/MAINTAINERS
> @@ -0,0 +1,4 @@
> +QEMU RAMFB VIDEO DRIVER
> +M: Alexander Graf <agraf@csgraf.de>
> +S: Maintained
> +F: drivers/video/ramfb.c
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 4038395b12..6cfec17072 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_VIDEO_TEGRA20) += tegra.o
> obj-$(CONFIG_VIDEO_VCXK) += bus_vcxk.o
> obj-$(CONFIG_VIDEO_VESA) += vesa.o
> obj-$(CONFIG_VIDEO_SEPS525) += seps525.o
> +obj-$(CONFIG_VIDEO_RAMFB) += ramfb.o
>
> obj-y += bridge/
> obj-y += sunxi/
> diff --git a/drivers/video/ramfb.c b/drivers/video/ramfb.c
> new file mode 100644
> index 0000000000..c46bfa3baa
> --- /dev/null
> +++ b/drivers/video/ramfb.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2022 Alexander Graf
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <log.h>
> +#include <video.h>
> +#include <asm/global_data.h>
> +#include <efi_loader.h>
> +#include <qfw.h>
> +
> +#define fourcc_code(a, b, c, d) ((u32)(a) | ((u32)(b) << 8) | \
> + ((u32)(c) << 16) | ((u32)(d) << 24))
> +#define DRM_FORMAT_XRGB8888 fourcc_code('X', 'R', '2', '4')
> +
> +#define DEFAULT_WIDTH 1280
> +#define DEFAULT_HEIGHT 900
> +#define DEFAULT_BPIX VIDEO_BPP32
> +#define DEFAULT_FORMAT VIDEO_X8R8G8B8
> +
> +struct ramfb_cfg {

Should that be in a qemu header file somewhere? Anyway, please add comments.

> + u64 addr;
> + u32 fourcc;
> + u32 flags;
> + u32 width;
> + u32 height;
> + u32 stride;
> +} __packed;
> +
> +static int ramfb_probe(struct udevice *dev)
> +{
> + struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> + struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> + u32 selector;
> + u64 base;
> + u64 size;
> + efi_status_t ret;
> + struct fw_file *file;
> + struct udevice *qfw;
> + struct dm_qfw_ops *ops;
> + struct qfw_dma dma = {};
> + struct ramfb_cfg cfg = {
> + .addr = 0,
> + .fourcc = cpu_to_be32(DRM_FORMAT_XRGB8888),
> + .flags = 0,
> + .width = cpu_to_be32(DEFAULT_WIDTH),
> + .height = cpu_to_be32(DEFAULT_HEIGHT),
> + .stride = 0,
> + };
> +
> + ret = qfw_get_dev(&qfw);
> + if (ret)
> + return -EPROBE_DEFER;
> +
> + ops = dm_qfw_get_ops(qfw);
> + if (!ops)
> + return -EPROBE_DEFER;
> +
> + file = qfw_find_file(qfw, "etc/ramfb");
> + if (!file) {
> + /* No ramfb available. At least we tried. */
> + return -ENOENT;
> + }
> +
> + size = DEFAULT_WIDTH * DEFAULT_HEIGHT * VNBYTES(DEFAULT_BPIX);
> + ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> + EFI_RESERVED_MEMORY_TYPE,
> + efi_size_in_pages(size), &base);

Please see video-uclass.c for how to to allocate the frame buffer.
This has nothing to do with EFI.

> + if (ret != EFI_SUCCESS)
> + return -ENOMEM;
> +
> + debug("%s: base=%llx, size=%llu\n", __func__, base, size);
> +
> + cfg.addr = cpu_to_be64(base);
> + plat->base = base;
> + plat->size = size;
> + uc_priv->xsize = DEFAULT_WIDTH;
> + uc_priv->ysize = DEFAULT_HEIGHT;
> + uc_priv->bpix = DEFAULT_BPIX;
> + uc_priv->format = DEFAULT_FORMAT;
> + uc_priv->fb = (void *)base;
> + uc_priv->fb_size = size;
> +
> + selector = be16_to_cpu(file->cfg.select);
> + dma.length = cpu_to_be32(sizeof(cfg));
> + dma.address = cpu_to_be64((uintptr_t)&cfg);
> + dma.control = cpu_to_be32(FW_CFG_DMA_WRITE | FW_CFG_DMA_SELECT |
> + (selector << 16));
> +
> + barrier();
> +
> + /* Send a DMA write request which enables the screen */
> + ops->read_entry_dma(qfw, &dma);
> +
> + return 0;
> +}
> +
> +U_BOOT_DRIVER(ramfb) = {
> + .name = "ramfb",
> + .id = UCLASS_VIDEO,
> + .probe = ramfb_probe,

You need a bind() method too.

> +};
> --
> 2.32.0
>

Regards,
Simon