Re: [PATCH 2/3] sample: vfio mdev display - guest driver

From: Alex Williamson
Date: Mon Apr 23 2018 - 22:51:25 EST


On Mon, 9 Apr 2018 12:35:12 +0200
Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:

> Guest fbdev driver for CONFIG_SAMPLE_VFIO_MDEV_MDPY.
>
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> samples/vfio-mdev/mdpy-fb.c | 232 ++++++++++++++++++++++++++++++++++++++++++++
> samples/Kconfig | 9 ++
> samples/vfio-mdev/Makefile | 1 +
> 3 files changed, 242 insertions(+)
> create mode 100644 samples/vfio-mdev/mdpy-fb.c

Looks good to me, just some trivial checkpatch issues below.

> diff --git a/samples/vfio-mdev/mdpy-fb.c b/samples/vfio-mdev/mdpy-fb.c
> new file mode 100644
> index 0000000000..0ebd8feead
> --- /dev/null
> +++ b/samples/vfio-mdev/mdpy-fb.c
> @@ -0,0 +1,232 @@
> +/*

SPDX license

> + * Framebuffer driver for mdpy (mediated virtual pci display device).
> + *
> + * See mdpy-defs.h for device specs
> + *
> + * (c) Gerd Hoffmann <kraxel@xxxxxxxxxx>
> + *
> + * Using some code snippets from simplefb and cirrusfb.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +#include <linux/errno.h>
> +#include <linux/fb.h>
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/module.h>
> +#include <drm/drm_fourcc.h>
> +#include "mdpy-defs.h"
> +
> +static const struct fb_fix_screeninfo mdpy_fb_fix = {
> + .id = "mdpy-fb",
> + .type = FB_TYPE_PACKED_PIXELS,
> + .visual = FB_VISUAL_TRUECOLOR,
> + .accel = FB_ACCEL_NONE,
> +};
> +
> +static const struct fb_var_screeninfo mdpy_fb_var = {
> + .height = -1,
> + .width = -1,
> + .activate = FB_ACTIVATE_NOW,
> + .vmode = FB_VMODE_NONINTERLACED,
> +
> + .bits_per_pixel = 32,
> + .transp.offset = 24,
> + .red.offset = 16,
> + .green.offset = 8,
> + .blue.offset = 0,
> + .transp.length = 8,
> + .red.length = 8,
> + .green.length = 8,
> + .blue.length = 8,
> +};
> +
> +#define PSEUDO_PALETTE_SIZE 16
> +
> +struct mdpy_fb_par {
> + u32 palette[PSEUDO_PALETTE_SIZE];
> +};
> +
> +static int mdpy_fb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
> + u_int transp, struct fb_info *info)
> +{
> + u32 *pal = info->pseudo_palette;
> + u32 cr = red >> (16 - info->var.red.length);
> + u32 cg = green >> (16 - info->var.green.length);
> + u32 cb = blue >> (16 - info->var.blue.length);
> + u32 value;
> +
> + if (regno >= PSEUDO_PALETTE_SIZE)
> + return -EINVAL;
> +
> + value = (cr << info->var.red.offset) |
> + (cg << info->var.green.offset) |
> + (cb << info->var.blue.offset);
> + if (info->var.transp.length > 0) {
> + u32 mask = (1 << info->var.transp.length) - 1;

Technically wants a new line here, I'm torn whether I'd ignore that
warning.

> + mask <<= info->var.transp.offset;
> + value |= mask;
> + }
> + pal[regno] = value;
> +
> + return 0;
> +}
> +
> +static void mdpy_fb_destroy(struct fb_info *info)
> +{
> + if (info->screen_base)
> + iounmap(info->screen_base);
> +}
> +
> +static struct fb_ops mdpy_fb_ops = {
> + .owner = THIS_MODULE,
> + .fb_destroy = mdpy_fb_destroy,
> + .fb_setcolreg = mdpy_fb_setcolreg,
> + .fb_fillrect = cfb_fillrect,
> + .fb_copyarea = cfb_copyarea,
> + .fb_imageblit = cfb_imageblit,
> +};
> +
> +static int mdpy_fb_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct fb_info *info;
> + struct mdpy_fb_par *par;
> + u32 format, width, height;
> + int ret;
> +
> + ret = pci_enable_device(pdev);
> + if (ret < 0)
> + return ret;
> +
> + ret = pci_request_regions(pdev, "mdpy-fb");
> + if (ret < 0)
> + return ret;
> +
> + pci_read_config_dword(pdev, MDPY_FORMAT_OFFSET, &format);
> + pci_read_config_dword(pdev, MDPY_WIDTH_OFFSET, &width);
> + pci_read_config_dword(pdev, MDPY_HEIGHT_OFFSET, &height);
> + if (format != DRM_FORMAT_XRGB8888) {
> + dev_err(&pdev->dev, "format mismatch (0x%x != 0x%x)\n",
> + format, DRM_FORMAT_XRGB8888);
> + return -EINVAL;
> + }
> + if (width < 100 || width > 10000) {
> + dev_err(&pdev->dev, "width (%d) out of range\n", width);
> + return -EINVAL;
> + }
> + if (height < 100 || height > 10000) {
> + dev_err(&pdev->dev, "height (%d) out of range\n", height);
> + return -EINVAL;
> + }
> + dev_info(&pdev->dev, "mdpy found: %dx%d framebuffer\n",
> + width, height);
> +
> + info = framebuffer_alloc(sizeof(struct mdpy_fb_par), &pdev->dev);
> + if (!info)
> + goto err_release_regions;
> + pci_set_drvdata(pdev, info);
> + par = info->par;
> +
> + info->fix = mdpy_fb_fix;
> + info->fix.smem_start = pci_resource_start(pdev, 0);
> + info->fix.smem_len = pci_resource_len(pdev, 0);
> + info->fix.line_length = width * 4;
> +
> + info->var = mdpy_fb_var;
> + info->var.xres = width;
> + info->var.yres = height;
> + info->var.xres_virtual = width;
> + info->var.yres_virtual = height;
> +
> + info->screen_size = info->fix.smem_len;
> + info->screen_base = ioremap(info->fix.smem_start,
> + info->screen_size);
> + if (!info->screen_base) {
> + dev_err(&pdev->dev, "ioremap(pcibar) failed\n");
> + ret = -EIO;
> + goto err_release_fb;
> + }
> +
> + info->apertures = alloc_apertures(1);
> + if (!info->apertures) {
> + ret = -ENOMEM;
> + goto err_unmap;
> + }
> + info->apertures->ranges[0].base = info->fix.smem_start;
> + info->apertures->ranges[0].size = info->fix.smem_len;
> +
> + info->fbops = &mdpy_fb_ops;
> + info->flags = FBINFO_DEFAULT;
> + info->pseudo_palette = par->palette;
> +
> + ret = register_framebuffer(info);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "mdpy-fb device register failed: %d\n", ret);
> + goto err_unmap;
> + }
> +
> + dev_info(&pdev->dev, "fb%d registered\n", info->node);
> + return 0;
> +
> +err_unmap:
> + iounmap(info->screen_base);
> +
> +err_release_fb:
> + framebuffer_release(info);
> +
> +err_release_regions:
> + pci_release_regions(pdev);
> +
> + return ret;
> +}
> +
> +static void mdpy_fb_remove(struct pci_dev *pdev)
> +{
> + struct fb_info *info = pci_get_drvdata(pdev);
> +
> + unregister_framebuffer(info);
> + framebuffer_release(info);
> +}
> +
> +static struct pci_device_id mdpy_fb_pci_table[] = {
> + {
> + .vendor = MDPY_PCI_VENDOR_ID,
> + .device = MDPY_PCI_DEVICE_ID,
> + .subvendor = MDPY_PCI_SUBVENDOR_ID,
> + .subdevice = MDPY_PCI_SUBDEVICE_ID,
> + },{

Space after comma, as in the first patch this is the only error from
checkpatch. Thanks,

Alex

> + /* end of list */
> + }
> +};
> +
> +static struct pci_driver mdpy_fb_pci_driver = {
> + .name = "mdpy-fb",
> + .id_table = mdpy_fb_pci_table,
> + .probe = mdpy_fb_probe,
> + .remove = mdpy_fb_remove,
> +};
> +
> +static int __init mdpy_fb_init(void)
> +{
> + int ret;
> +
> + ret = pci_register_driver(&mdpy_fb_pci_driver);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +module_init(mdpy_fb_init);
> +
> +MODULE_DEVICE_TABLE(pci, mdpy_fb_pci_table);
> +MODULE_LICENSE("GPL v2");
> diff --git a/samples/Kconfig b/samples/Kconfig
> index a0c104adda..755430c788 100644
> --- a/samples/Kconfig
> +++ b/samples/Kconfig
> @@ -119,6 +119,15 @@ config SAMPLE_VFIO_MDEV_MDPY
> mediated device. It is a simple framebuffer and supports
> the region display interface (VFIO_GFX_PLANE_TYPE_REGION).
>
> +config SAMPLE_VFIO_MDEV_MDPY_FB
> + tristate "Build VFIO mdpy example guest fbdev driver -- loadable module only"
> + depends on FB && m
> + select FB_CFB_FILLRECT
> + select FB_CFB_COPYAREA
> + select FB_CFB_IMAGEBLIT
> + help
> + Guest fbdev driver for the virtual display sample driver.
> +
> config SAMPLE_STATX
> bool "Build example extended-stat using code"
> depends on BROKEN
> diff --git a/samples/vfio-mdev/Makefile b/samples/vfio-mdev/Makefile
> index 031d6b88e9..7a5790aaec 100644
> --- a/samples/vfio-mdev/Makefile
> +++ b/samples/vfio-mdev/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_SAMPLE_VFIO_MDEV_MTTY) += mtty.o
> obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY) += mdpy.o
> +obj-$(CONFIG_SAMPLE_VFIO_MDEV_MDPY_FB) += mdpy-fb.o