Re: [PATCH] drm/virtio: switch to generic fbdev emulation
On Thu, 2018-12-13 at 14:49 +0100, Gerd Hoffmann wrote: A few words explaining why the generic emulation is OK would be useful. The commit might be clear for seasoned DRM developers, however given this driver is a nice target for those learning DRM (as it can be tested easily), I think having a more verbose history is useful. The patch looks good: Reviewed-by: Ezequiel Garcia > Signed-off-by: Gerd Hoffmann > --- > drivers/gpu/drm/virtio/virtgpu_drv.h | 14 --- > drivers/gpu/drm/virtio/virtgpu_display.c | 1 - > drivers/gpu/drm/virtio/virtgpu_drv.c | 9 +- > drivers/gpu/drm/virtio/virtgpu_fb.c | 191 > --- > drivers/gpu/drm/virtio/virtgpu_kms.c | 8 -- > 5 files changed, 8 insertions(+), 215 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h > b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 1deb41d42e..63704915f8 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -137,19 +137,10 @@ struct virtio_gpu_framebuffer { > #define to_virtio_gpu_framebuffer(x) \ > container_of(x, struct virtio_gpu_framebuffer, base) > > -struct virtio_gpu_fbdev { > - struct drm_fb_helper helper; > - struct virtio_gpu_framebuffer vgfb; > - struct virtio_gpu_device *vgdev; > - struct delayed_workwork; > -}; > - > struct virtio_gpu_mman { > struct ttm_bo_devicebdev; > }; > > -struct virtio_gpu_fbdev; > - > struct virtio_gpu_queue { > struct virtqueue *vq; > spinlock_t qlock; > @@ -180,8 +171,6 @@ struct virtio_gpu_device { > > struct virtio_gpu_mman mman; > > - /* pointer to fbdev info structure */ > - struct virtio_gpu_fbdev *vgfbdev; > struct virtio_gpu_output outputs[VIRTIO_GPU_MAX_SCANOUTS]; > uint32_t num_scanouts; > > @@ -249,9 +238,6 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv, > uint32_t handle, uint64_t *offset_p); > > /* virtio_fb */ > -#define VIRTIO_GPUFB_CONN_LIMIT 1 > -int virtio_gpu_fbdev_init(struct virtio_gpu_device *vgdev); > -void virtio_gpu_fbdev_fini(struct virtio_gpu_device *vgdev); > int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb, >struct drm_clip_rect *clips, >unsigned int num_clips); > diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c > b/drivers/gpu/drm/virtio/virtgpu_display.c > index b5580b11a0..e1c223e18d 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_display.c > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c > @@ -390,6 +390,5 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device > *vgdev) > > for (i = 0 ; i < vgdev->num_scanouts; ++i) > kfree(vgdev->outputs[i].edid); > - virtio_gpu_fbdev_fini(vgdev); > drm_mode_config_cleanup(vgdev->ddev); > } > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c > b/drivers/gpu/drm/virtio/virtgpu_drv.c > index f7f32a885a..7df50920c1 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.c > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c > @@ -42,13 +42,20 @@ module_param_named(modeset, virtio_gpu_modeset, int, > 0400); > > static int virtio_gpu_probe(struct virtio_device *vdev) > { > + int ret; > + > if (vgacon_text_force() && virtio_gpu_modeset == -1) > return -EINVAL; > > if (virtio_gpu_modeset == 0) > return -EINVAL; > > - return drm_virtio_init(&driver, vdev); > + ret = drm_virtio_init(&driver, vdev); > + if (ret) > + return ret; > + > + drm_fbdev_generic_setup(vdev->priv, 32); > + return 0; > } > > static void virtio_gpu_remove(struct virtio_device *vdev) > diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c > b/drivers/gpu/drm/virtio/virtgpu_fb.c > index fb1cc8b2f1..b07584b1c2 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_fb.c > +++ b/drivers/gpu/drm/virtio/virtgpu_fb.c > @@ -27,8 +27,6 @@ > #include > #include "virtgpu_drv.h" > > -#define VIRTIO_GPU_FBCON_POLL_PERIOD (HZ / 60) > - > static int virtio_gpu_dirty_update(struct virtio_gpu_framebuffer *fb, > bool store, int x, int y, > int width, int height) > @@ -150,192 +148,3 @@ int virtio_gpu_surface_dirty(struct > virtio_gpu_framebuffer *vgfb, > left, top, right - left, bottom - top); > return 0; > } > - > -static void virtio_gpu_fb_dirty_work(struct work_struct *work) > -{ > - struct delayed_work *delayed_work = to_delayed_work(work); > - struct virtio_gpu_fbdev *vfbdev = > - container_of(delayed_work, struct virtio_gpu_fbdev, work); > - struct virtio_gpu_framebuffer *vgfb = &vfbdev->vgfb; > - > - virtio_gpu_dirty_update(&vfbdev->vgfb, false, vgfb->x1, vgfb->y1, > - vgfb->x2 - vgfb->x1, vgfb->y2 - vgfb->y1); > -} > - > -static void virtio_gpu_3d_fillrect(struct fb_
Re: [PATCH] drm/virtio: switch to generic fbdev emulation
On Thu, 13 Dec 2018 at 23:49, Gerd Hoffmann wrote: > > Signed-off-by: Gerd Hoffmann Seems correct to me, Reviewed-by: Dave Airlie
[PATCH] drm/virtio: switch to generic fbdev emulation
Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_drv.h | 14 --- drivers/gpu/drm/virtio/virtgpu_display.c | 1 - drivers/gpu/drm/virtio/virtgpu_drv.c | 9 +- drivers/gpu/drm/virtio/virtgpu_fb.c | 191 --- drivers/gpu/drm/virtio/virtgpu_kms.c | 8 -- 5 files changed, 8 insertions(+), 215 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 1deb41d42e..63704915f8 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -137,19 +137,10 @@ struct virtio_gpu_framebuffer { #define to_virtio_gpu_framebuffer(x) \ container_of(x, struct virtio_gpu_framebuffer, base) -struct virtio_gpu_fbdev { - struct drm_fb_helper helper; - struct virtio_gpu_framebuffer vgfb; - struct virtio_gpu_device *vgdev; - struct delayed_workwork; -}; - struct virtio_gpu_mman { struct ttm_bo_devicebdev; }; -struct virtio_gpu_fbdev; - struct virtio_gpu_queue { struct virtqueue *vq; spinlock_t qlock; @@ -180,8 +171,6 @@ struct virtio_gpu_device { struct virtio_gpu_mman mman; - /* pointer to fbdev info structure */ - struct virtio_gpu_fbdev *vgfbdev; struct virtio_gpu_output outputs[VIRTIO_GPU_MAX_SCANOUTS]; uint32_t num_scanouts; @@ -249,9 +238,6 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv, uint32_t handle, uint64_t *offset_p); /* virtio_fb */ -#define VIRTIO_GPUFB_CONN_LIMIT 1 -int virtio_gpu_fbdev_init(struct virtio_gpu_device *vgdev); -void virtio_gpu_fbdev_fini(struct virtio_gpu_device *vgdev); int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *qfb, struct drm_clip_rect *clips, unsigned int num_clips); diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index b5580b11a0..e1c223e18d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -390,6 +390,5 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev) for (i = 0 ; i < vgdev->num_scanouts; ++i) kfree(vgdev->outputs[i].edid); - virtio_gpu_fbdev_fini(vgdev); drm_mode_config_cleanup(vgdev->ddev); } diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c index f7f32a885a..7df50920c1 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.c +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c @@ -42,13 +42,20 @@ module_param_named(modeset, virtio_gpu_modeset, int, 0400); static int virtio_gpu_probe(struct virtio_device *vdev) { + int ret; + if (vgacon_text_force() && virtio_gpu_modeset == -1) return -EINVAL; if (virtio_gpu_modeset == 0) return -EINVAL; - return drm_virtio_init(&driver, vdev); + ret = drm_virtio_init(&driver, vdev); + if (ret) + return ret; + + drm_fbdev_generic_setup(vdev->priv, 32); + return 0; } static void virtio_gpu_remove(struct virtio_device *vdev) diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c index fb1cc8b2f1..b07584b1c2 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fb.c +++ b/drivers/gpu/drm/virtio/virtgpu_fb.c @@ -27,8 +27,6 @@ #include #include "virtgpu_drv.h" -#define VIRTIO_GPU_FBCON_POLL_PERIOD (HZ / 60) - static int virtio_gpu_dirty_update(struct virtio_gpu_framebuffer *fb, bool store, int x, int y, int width, int height) @@ -150,192 +148,3 @@ int virtio_gpu_surface_dirty(struct virtio_gpu_framebuffer *vgfb, left, top, right - left, bottom - top); return 0; } - -static void virtio_gpu_fb_dirty_work(struct work_struct *work) -{ - struct delayed_work *delayed_work = to_delayed_work(work); - struct virtio_gpu_fbdev *vfbdev = - container_of(delayed_work, struct virtio_gpu_fbdev, work); - struct virtio_gpu_framebuffer *vgfb = &vfbdev->vgfb; - - virtio_gpu_dirty_update(&vfbdev->vgfb, false, vgfb->x1, vgfb->y1, - vgfb->x2 - vgfb->x1, vgfb->y2 - vgfb->y1); -} - -static void virtio_gpu_3d_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct virtio_gpu_fbdev *vfbdev = info->par; - - drm_fb_helper_sys_fillrect(info, rect); - virtio_gpu_dirty_update(&vfbdev->vgfb, true, rect->dx, rect->dy, -rect->width, rect->height); - schedule_delayed_work(&vfbdev->work, VIRTIO_GPU_FBCON_POLL_PERIOD); -} - -static void virtio_gpu_3d_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct virtio_gpu_fbdev *vf