Hi I only have a number of trivial changes listed below. With these fixed: Acked-by: Thomas Zimmermann Am 17.05.21 um 13:59 schrieb Deepak Rawat: > DRM driver for hyperv synthetic video device, based on hyperv_fb > framebuffer driver. Also added config option "DRM_HYPERV" to enabled > this driver. > > v2: > - Add support for gen2 VM > - Fixed review comments > > v3: > - Split into multiple files as suggested by Thomas Zimmermann > - Fixed hibernation issue as suggested by Dexuan Cui > - Use ioremap_cache as suggested by Dexuan Cui > - Incorporated other review comments > > v4: > - Fix bitrotted code > - Review comments > - Updated the copyright and license to match hyperv_fb > > Signed-off-by: Deepak Rawat > --- > drivers/gpu/drm/Kconfig | 12 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/hyperv/Makefile | 8 + > drivers/gpu/drm/hyperv/hyperv_drm.h | 51 +++ > drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 310 +++++++++++++ > drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 238 ++++++++++ > drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 478 ++++++++++++++++++++ > 7 files changed, 1098 insertions(+) > create mode 100644 drivers/gpu/drm/hyperv/Makefile > create mode 100644 drivers/gpu/drm/hyperv/hyperv_drm.h > create mode 100644 drivers/gpu/drm/hyperv/hyperv_drm_drv.c > create mode 100644 drivers/gpu/drm/hyperv/hyperv_drm_modeset.c > create mode 100644 drivers/gpu/drm/hyperv/hyperv_drm_proto.c > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 3c16bd1afd87..15bb0165ebf9 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -386,6 +386,18 @@ source "drivers/gpu/drm/xlnx/Kconfig" > > source "drivers/gpu/drm/gud/Kconfig" > > +config DRM_HYPERV > + tristate "DRM Support for hyperv synthetic video device" > + depends on DRM && PCI && MMU && HYPERV > + select DRM_KMS_HELPER > + select DRM_GEM_SHMEM_HELPER > + help > + This is a KMS driver for hyperv synthetic video device. Choose this > + option if you would like to enable drm driver for Hyper-V virtual > + machine. > + > + If M is selected the module will be called hyperv_drm. > + > # Keep legacy drivers last > > menuconfig DRM_LEGACY > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 5279db4392df..d0ae4feaae33 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -126,3 +126,4 @@ obj-$(CONFIG_DRM_MCDE) += mcde/ > obj-$(CONFIG_DRM_TIDSS) += tidss/ > obj-y += xlnx/ > obj-y += gud/ > +obj-$(CONFIG_DRM_HYPERV) += hyperv/ > diff --git a/drivers/gpu/drm/hyperv/Makefile b/drivers/gpu/drm/hyperv/Makefile > new file mode 100644 > index 000000000000..0039b03db0be > --- /dev/null > +++ b/drivers/gpu/drm/hyperv/Makefile > @@ -0,0 +1,8 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +hyperv_drm-y := \ > + hyperv_drm_proto.o \ > + hyperv_drm_modeset.o \ > + hyperv_drm_drv.o Alphabetical sorting prefered. > + > +obj-$(CONFIG_DRM_HYPERV) += hyperv_drm.o > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm.h b/drivers/gpu/drm/hyperv/hyperv_drm.h > new file mode 100644 > index 000000000000..e1d1fdea96f2 > --- /dev/null > +++ b/drivers/gpu/drm/hyperv/hyperv_drm.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright 2021 Microsoft > + */ > + > +#ifndef _HYPERV_DRM_H_ > +#define _HYPERV_DRM_H_ > + > +#define VMBUS_MAX_PACKET_SIZE 0x4000 > + > +struct hyperv_drm_device { > + /* drm */ > + struct drm_device dev; > + struct drm_simple_display_pipe pipe; > + struct drm_connector connector; > + > + /* mode */ > + u32 screen_width_max; > + u32 screen_height_max; > + u32 preferred_width; > + u32 preferred_height; > + u32 screen_depth; > + > + /* hw */ > + struct resource *mem; > + void __iomem *vram; > + unsigned long fb_base; > + unsigned long fb_size; > + struct completion wait; > + u32 synthvid_version; > + u32 mmio_megabytes; > + > + u8 init_buf[VMBUS_MAX_PACKET_SIZE]; > + u8 recv_buf[VMBUS_MAX_PACKET_SIZE]; > + > + struct hv_device *hdev; > +}; > + > +#define to_hv(_dev) container_of(_dev, struct hyperv_drm_device, dev) > + > +/* hyperv_drm_modeset */ > +int hyperv_mode_config_init(struct hyperv_drm_device *hv); > + > +/* hyperv_drm_proto */ > +int hyperv_update_vram_location(struct hv_device *hdev, phys_addr_t vram_pp); > +int hyperv_update_situation(struct hv_device *hdev, u8 active, u32 bpp, > + u32 w, u32 h, u32 pitch); > +int hyperv_update_dirt(struct hv_device *hdev, struct drm_rect *rect); > +int hyperv_connect_vsp(struct hv_device *hdev); > + > +#endif > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c > new file mode 100644 > index 000000000000..68a6ba91a486 > --- /dev/null > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c > @@ -0,0 +1,310 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2021 Microsoft > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > + > +#include "hyperv_drm.h" > + > +#define DRIVER_NAME "hyperv_drm" > +#define DRIVER_DESC "DRM driver for hyperv synthetic video device" > +#define DRIVER_DATE "2020" > +#define DRIVER_MAJOR 1 > +#define DRIVER_MINOR 0 > + > +#define PCI_VENDOR_ID_MICROSOFT 0x1414 > +#define PCI_DEVICE_ID_HYPERV_VIDEO 0x5353 > + > +DEFINE_DRM_GEM_FOPS(hv_fops); > + > +static struct drm_driver hyperv_driver = { > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, > + > + .name = DRIVER_NAME, > + .desc = DRIVER_DESC, > + .date = DRIVER_DATE, > + .major = DRIVER_MAJOR, > + .minor = DRIVER_MINOR, > + > + .fops = &hv_fops, > + DRM_GEM_SHMEM_DRIVER_OPS, > +}; > + > +static int hyperv_pci_probe(struct pci_dev *pdev, > + const struct pci_device_id *ent) > +{ > + return 0; > +} > + > +static void hyperv_pci_remove(struct pci_dev *pdev) > +{ > +} > + > +static const struct pci_device_id hyperv_pci_tbl[] = { > + { > + .vendor = PCI_VENDOR_ID_MICROSOFT, > + .device = PCI_DEVICE_ID_HYPERV_VIDEO, > + }, > + { /* end of list */ } > +}; > + > +static struct pci_driver hyperv_pci_driver = { > + .name = KBUILD_MODNAME, > + .id_table = hyperv_pci_tbl, > + .probe = hyperv_pci_probe, > + .remove = hyperv_pci_remove, > +}; The PCI code doesn't do anything. Do you need this for gen1 to work corretly. If so, there should at least be a short comment. Why don't you call hyperv_setup_gen1() in the PCI probe function? > + > +static int hyperv_setup_gen1(struct hyperv_drm_device *hv) > +{ > + struct drm_device *dev = &hv->dev; > + struct pci_dev *pdev; > + int ret; > + > + pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT, > + PCI_DEVICE_ID_HYPERV_VIDEO, NULL); > + if (!pdev) { > + drm_err(dev, "Unable to find PCI Hyper-V video\n"); > + return -ENODEV; > + } > + > + ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "hypervdrmfb"); This function is no more. There's now drm_aperture_remove_conflicting_pci_framebuffers() from drm/drm_aperture.h Works the same way. You probably need to rebase onto the latest drm-misc-next or drm-tip tree. > + if (ret) { > + drm_err(dev, "Not able to remove boot fb\n"); > + return ret; > + } > + > + if (pci_request_region(pdev, 0, DRIVER_NAME) != 0) > + drm_warn(dev, "Cannot request framebuffer, boot fb still active?\n"); > + > + if ((pdev->resource[0].flags & IORESOURCE_MEM) == 0) { > + drm_err(dev, "Resource at bar 0 is not IORESOURCE_MEM\n"); > + ret = -ENODEV; > + goto error; > + } > + > + hv->fb_base = pci_resource_start(pdev, 0); > + hv->fb_size = pci_resource_len(pdev, 0); > + if (!hv->fb_base) { > + drm_err(dev, "Resource not available\n"); > + ret = -ENODEV; > + goto error; > + } > + > + hv->fb_size = min(hv->fb_size, > + (unsigned long)(hv->mmio_megabytes * 1024 * 1024)); > + hv->vram = devm_ioremap(&pdev->dev, hv->fb_base, hv->fb_size); > + if (!hv->vram) { > + drm_err(dev, "Failed to map vram\n"); > + ret = -ENOMEM; > + } > + > +error: > + pci_dev_put(pdev); > + return ret; > +} > + > +static int hyperv_setup_gen2(struct hyperv_drm_device *hv, > + struct hv_device *hdev) > +{ > + struct drm_device *dev = &hv->dev; > + struct apertures_struct *ap; > + int ret; > + > + ap = alloc_apertures(1); > + if (!ap) { > + drm_err(dev, "Failed to get apertures\n"); > + return -ENOMEM; > + } > + > + ap->ranges[0].base = screen_info.lfb_base; > + ap->ranges[0].size = screen_info.lfb_size; > + drm_fb_helper_remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false); For this function, there's now drm_aperture_remove_conflicting_framebuffers(). > + kfree(ap); > + > + hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024; > + > + ret = vmbus_allocate_mmio(&hv->mem, hdev, 0, -1, hv->fb_size, 0x100000, > + true); > + if (ret) { > + drm_err(dev, "Failed to allocate mmio\n"); > + return -ENOMEM; > + } > + > + /* > + * Map the VRAM cacheable for performance. This is also required for VM > + * connect to display properly for ARM64 Linux VM, as the host also maps > + * the VRAM cacheable. > + */ > + hv->vram = ioremap_cache(hv->mem->start, hv->fb_size); > + if (!hv->vram) { > + drm_err(dev, "Failed to map vram\n"); > + ret = -ENOMEM; > + goto error; > + } > + > + hv->fb_base = hv->mem->start; > + return 0; > + > +error: > + vmbus_free_mmio(hv->mem->start, hv->fb_size); > + return ret; > +} > + > +static int hyperv_vmbus_probe(struct hv_device *hdev, > + const struct hv_vmbus_device_id *dev_id) > +{ > + struct hyperv_drm_device *hv; > + struct drm_device *dev; > + int ret; > + > + hv = devm_drm_dev_alloc(&hdev->device, &hyperv_driver, > + struct hyperv_drm_device, dev); > + if (IS_ERR(hv)) > + return PTR_ERR(hv); > + > + dev = &hv->dev; > + init_completion(&hv->wait); > + hv_set_drvdata(hdev, hv); > + hv->hdev = hdev; > + > + ret = hyperv_connect_vsp(hdev); > + if (ret) { > + drm_err(dev, "Failed to connect to vmbus.\n"); > + goto err_hv_set_drv_data; > + } > + > + if (efi_enabled(EFI_BOOT)) > + ret = hyperv_setup_gen2(hv, hdev); > + else > + ret = hyperv_setup_gen1(hv); > + > + if (ret) > + goto err_vmbus_close; > + > + /* > + * Should be done only once during init and resume. Failing to update > + * vram location is not fatal. Device will update dirty area till > + * preferred resolution only. > + */ > + ret = hyperv_update_vram_location(hdev, hv->fb_base); > + if (ret) > + drm_warn(dev, "Failed to update vram location.\n"); > + > + ret = hyperv_mode_config_init(hv); > + if (ret) > + goto err_vmbus_close; > + > + ret = drm_dev_register(dev, 0); > + if (ret) { > + drm_err(dev, "Failed to register drm driver.\n"); > + goto err_vmbus_close; > + } > + > + drm_fbdev_generic_setup(dev, 0); > + > + return 0; > + > +err_vmbus_close: > + vmbus_close(hdev->channel); > +err_hv_set_drv_data: > + hv_set_drvdata(hdev, NULL); > + return ret; > +} > + > +static int hyperv_vmbus_remove(struct hv_device *hdev) > +{ > + struct drm_device *dev = hv_get_drvdata(hdev); > + struct hyperv_drm_device *hv = to_hv(dev); > + > + drm_dev_unplug(dev); > + drm_atomic_helper_shutdown(dev); > + vmbus_close(hdev->channel); > + hv_set_drvdata(hdev, NULL); > + vmbus_free_mmio(hv->mem->start, hv->fb_size); > + > + return 0; > +} > + > +static int hyperv_vmbus_suspend(struct hv_device *hdev) > +{ > + struct drm_device *dev = hv_get_drvdata(hdev); > + int ret; > + > + ret = drm_mode_config_helper_suspend(dev); > + > + vmbus_close(hdev->channel); > + > + return ret; > +} > + > +static int hyperv_vmbus_resume(struct hv_device *hdev) > +{ > + struct drm_device *dev = hv_get_drvdata(hdev); > + struct hyperv_drm_device *hv = to_hv(dev); > + int ret; > + > + ret = hyperv_connect_vsp(hdev); > + if (ret) > + return ret; > + > + ret = hyperv_update_vram_location(hdev, hv->fb_base); > + if (ret) > + return ret; > + > + return drm_mode_config_helper_resume(dev); > +} > + > +static const struct hv_vmbus_device_id hyperv_vmbus_tbl[] = { > + /* Synthetic Video Device GUID */ > + {HV_SYNTHVID_GUID}, > + {} > +}; > + > +static struct hv_driver hyperv_hv_driver = { > + .name = KBUILD_MODNAME, > + .id_table = hyperv_vmbus_tbl, > + .probe = hyperv_vmbus_probe, > + .remove = hyperv_vmbus_remove, > + .suspend = hyperv_vmbus_suspend, > + .resume = hyperv_vmbus_resume, > + .driver = { > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > + }, > +}; > + > +static int __init hyperv_init(void) > +{ > + int ret; > + > + ret = pci_register_driver(&hyperv_pci_driver); > + if (ret != 0) > + return ret; > + > + return vmbus_driver_register(&hyperv_hv_driver); > +} > + > +static void __exit hyperv_exit(void) > +{ > + vmbus_driver_unregister(&hyperv_hv_driver); > + pci_unregister_driver(&hyperv_pci_driver); > +} > + > +module_init(hyperv_init); > +module_exit(hyperv_exit); > + > +MODULE_DEVICE_TABLE(pci, hyperv_pci_tbl); > +MODULE_DEVICE_TABLE(vmbus, hyperv_vmbus_tbl); > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Deepak Rawat "); > +MODULE_DESCRIPTION("DRM driver for hyperv synthetic video device"); > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c > new file mode 100644 > index 000000000000..5d608472d294 > --- /dev/null > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c > @@ -0,0 +1,238 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2021 Microsoft > + */ > + > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "hyperv_drm.h" > + > +static int hyperv_blit_to_vram_rect(struct drm_framebuffer *fb, > + const struct dma_buf_map *map, > + struct drm_rect *rect) > +{ > + struct hyperv_drm_device *hv = to_hv(fb->dev); > + void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */ > + int idx; > + > + if (!drm_dev_enter(&hv->dev, &idx)) > + return -ENODEV; > + > + drm_fb_memcpy_dstclip(hv->vram, vmap, fb, rect); > + drm_dev_exit(idx); > + > + return 0; > +} > + > +static int hyperv_blit_to_vram_fullscreen(struct drm_framebuffer *fb, const struct dma_buf_map *map) > +{ > + struct drm_rect fullscreen = { > + .x1 = 0, > + .x2 = fb->width, > + .y1 = 0, > + .y2 = fb->height, > + }; > + return hyperv_blit_to_vram_rect(fb, map, &fullscreen); > +} > + > +static int hyperv_connector_get_modes(struct drm_connector *connector) > +{ > + struct hyperv_drm_device *hv = to_hv(connector->dev); > + int count; > + > + count = drm_add_modes_noedid(connector, > + connector->dev->mode_config.max_width, > + connector->dev->mode_config.max_height); > + drm_set_preferred_mode(connector, hv->preferred_width, > + hv->preferred_height); > + > + return count; > +} > + > +static const struct drm_connector_helper_funcs hyperv_connector_helper_funcs = { > + .get_modes = hyperv_connector_get_modes, > +}; > + > +static const struct drm_connector_funcs hyperv_connector_funcs = { > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = drm_connector_cleanup, > + .reset = drm_atomic_helper_connector_reset, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; > + > +static inline int hyperv_conn_init(struct hyperv_drm_device *hv) > +{ > + drm_connector_helper_add(&hv->connector, &hyperv_connector_helper_funcs); > + return drm_connector_init(&hv->dev, &hv->connector, > + &hyperv_connector_funcs, > + DRM_MODE_CONNECTOR_VIRTUAL); > +} > + > +static int hyperv_check_size(struct hyperv_drm_device *hv, int w, int h, > + struct drm_framebuffer *fb) > +{ > + u32 pitch = w * (hv->screen_depth / 8); > + > + if (fb) > + pitch = fb->pitches[0]; > + > + if (pitch * h > hv->fb_size) > + return -EINVAL; > + > + return 0; > +} > + > +static void hyperv_pipe_enable(struct drm_simple_display_pipe *pipe, > + struct drm_crtc_state *crtc_state, > + struct drm_plane_state *plane_state) > +{ > + struct hyperv_drm_device *hv = to_hv(pipe->crtc.dev); > + struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state); > + > + hyperv_update_situation(hv->hdev, 1, hv->screen_depth, > + crtc_state->mode.hdisplay, > + crtc_state->mode.vdisplay, > + plane_state->fb->pitches[0]); > + hyperv_blit_to_vram_fullscreen(plane_state->fb, &shadow_plane_state->map[0]); > +} > + > +static int hyperv_pipe_check(struct drm_simple_display_pipe *pipe, > + struct drm_plane_state *plane_state, > + struct drm_crtc_state *crtc_state) > +{ > + struct hyperv_drm_device *hv = to_hv(pipe->crtc.dev); > + struct drm_framebuffer *fb = plane_state->fb; > + > + if (fb->format->format != DRM_FORMAT_XRGB8888) > + return -EINVAL; > + > + if (fb->pitches[0] * fb->height > hv->fb_size) > + return -EINVAL; > + > + return 0; > +} > + > +static void hyperv_pipe_update(struct drm_simple_display_pipe *pipe, > + struct drm_plane_state *old_state) > +{ > + struct hyperv_drm_device *hv = to_hv(pipe->crtc.dev); > + struct drm_plane_state *state = pipe->plane.state; > + struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state); > + struct drm_rect rect; > + > + if (drm_atomic_helper_damage_merged(old_state, state, &rect)) { > + hyperv_blit_to_vram_rect(state->fb, &shadow_plane_state->map[0], &rect); > + hyperv_update_dirt(hv->hdev, &rect); > + } > +} > + > +static const struct drm_simple_display_pipe_funcs hyperv_pipe_funcs = { > + .enable = hyperv_pipe_enable, > + .check = hyperv_pipe_check, > + .update = hyperv_pipe_update, > + DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS, > +}; > + > +static const uint32_t hyperv_formats[] = { > + DRM_FORMAT_XRGB8888, > +}; > + > +static const uint64_t hyperv_modifiers[] = { > + DRM_FORMAT_MOD_LINEAR, > + DRM_FORMAT_MOD_INVALID > +}; > + > +static inline int hyperv_pipe_init(struct hyperv_drm_device *hv) > +{ > + int ret; > + > + ret = drm_simple_display_pipe_init(&hv->dev, > + &hv->pipe, > + &hyperv_pipe_funcs, > + hyperv_formats, > + ARRAY_SIZE(hyperv_formats), > + NULL, > + &hv->connector); > + if (ret) > + return ret; > + > + drm_plane_enable_fb_damage_clips(&hv->pipe.plane); > + > + return 0; > +} > + > +static struct drm_framebuffer* > +hyperv_fb_create(struct drm_device *dev, struct drm_file *file_priv, > + const struct drm_mode_fb_cmd2 *mode_cmd) > +{ > + return drm_gem_fb_create_with_dirty(dev, file_priv, mode_cmd); No pointless wrappers, please. Use this function directly. Best regards Thomas -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer